Laravel: decoupled from SDK ClockFactory (& some linting) #211
Conversation
Use 1.0 stable release for `open-telemetry/api`.
| @@ -12,17 +12,15 @@ | |||
| "ext-json": "*", | |||
| "ext-opentelemetry": "*", | |||
| "laravel/framework": ">=6.0", | |||
| "open-telemetry/api": "^1.0.0beta10", | |||
| "open-telemetry/api": "^1.0", | |||
| "open-telemetry/sdk": "^1.0", | |||
There was a problem hiding this comment.
Required for:
There was a problem hiding this comment.
Is there a way for it to not directly use the SDK? The OTEL spec is fairly firm about it: https://opentelemetry.io/docs/specs/otel/library-guidelines/#requirements
There was a problem hiding this comment.
Thanks for pointing it out, I was not aware. Given that rule, it looks like the current implementation might be best in not using the ClockFactory, if possible?
I shall take a look at removing that coupling.
There was a problem hiding this comment.
I'm happy with your changes. We could maybe move ClockInterface into the API (since it has some constants that might make that code a little more readable), but it's a pretty minor use-case and nothing else in contrib needs if, AFAICT.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #211 +/- ##
=========================================
Coverage 33.08% 33.08%
Complexity 853 853
=========================================
Files 76 76
Lines 3252 3252
=========================================
Hits 1076 1076
Misses 2176 2176
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
…rom the runtime dependency on the ClockFactory.
ChrisLightfootWild
changed the title
OpenTelemetry\SDK is used in the QueryWatcher instrumentation, so has been promoted from a dev dependency.