✅ Deploy Preview for polite-licorice-3db33c canceled.

Codecov Report

Attention: 34 lines in your changes are missing coverage. Please review.

Comparison is base (55dc8cf) 73.53% compared to head (ab8020e) 73.04%.

@@            Coverage Diff             @@
##             main    #1142      +/-   ##
==========================================
- Coverage   73.53%   73.04%   -0.49%     
==========================================
  Files          32       32              
  Lines        3113     3135      +22     
==========================================
+ Hits         2289     2290       +1     
- Misses        717      740      +23     
+ Partials      107      105       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Full transparency, I'm still relatively new to golang, so please take everything I've written here with a grain of salt.

That said, I think this is in a decent place for a first round of reviews.

Areas of concern/in need of feedback:

• Should telemetry wire-up ever return errors?
  • OTel SDKs tend to err on the side of never break the app no matter what
  • Existing behavior returns errors in some cases.
  • But on some new code paths I've opted to log debug messages instead of returning errors, based on the rationale that if we're deferring to the OTel SDK for wire-up, then we should defer to its style of graceful failure too.
  • Mostly asking this question for my edification in the event that this is something the project already has a strong opinion on.
• The tests are passing...but I also may have broken the intent of one or more of the tests.
  • Some of the tests were related to the above should we error question, so just looking for feedback here on what the project/org expects in terms of coverage/quality in tests (on both fronts, I expect what is currently here will probably need some work).
• Do we/anyone have a well-defined/formal integration environment? Are the unit tests that environment? How would we know if I've borked something subtle here?

Anyways, please have at it and let me know what you think 🙇

(and again, I'm still new to golang, so if you see garbage code, please call it out for being garbage code, rather get the feedback sooner rather than later lol)

I think this is a good idea, but I would prefer to make this a non-breaking change. We can mark the arguments as deprecated and completely remove them at a later point. I think we can make the existing arguments and environment variables into the official OTel environment variables.

I think this is a good idea, but I would prefer to make this a non-breaking change. We can mark the arguments as deprecated and completely remove them at a later point. I think we can make the existing arguments and environment variables into the official OTel environment variables.

Okay, that makes sense. I started this PR just as a sample to gauge interest, so went in axe swinging, but can definitely scale back the frontend changes.

I'll take another pass to bring those frontend flags/envs back and try to get everything aligned with @thisthat's comments in #1141 (comment).

FYI waylaid by some other priorities, but this is still on my radar.