-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Introduce service.Settings.TelemetryFactory #13838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
902c8dc
to
875114a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13838 +/- ##
==========================================
- Coverage 91.66% 91.66% -0.01%
==========================================
Files 652 653 +1
Lines 42520 42551 +31
==========================================
+ Hits 38978 39006 +28
- Misses 2736 2739 +3
Partials 806 806 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
875114a
to
85d7247
Compare
The `service.Settings` type now has a `TelemetryFactory` field for injecting the `telemetry.Factory` to be used for creating a logger and logger provider, meter provider, and tracer provider. The `otelcol` package is hard-coded to inject an otelconftelemetry factory for now. In a followup we will make it possible to inject the telemetry through `otelcol.Factories`.
85d7247
to
872b902
Compare
# Use this changelog template to create an entry for release notes. | ||
|
||
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
change_type: breaking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to reviewer: I've put this down as a breaking API change because the new setting is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before moving forward, I really want to first fix #4970 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically we split the test package(s) into a separate module. Does it make sense to do that here?
Description
The
service.Settings
type now has aTelemetryFactory
field for injecting thetelemetry.Factory
to be used for creating a logger and logger provider, meter provider, and tracer provider. Theotelcol
package is hard-coded to inject anotelconftelemetry
factory for now. In a followup we will make it possible to inject the telemetry throughotelcol.Factories
.Link to tracking issue
Part of #4970
Testing
Manually verified that service telemetry configuration still works with a collector binary.
Documentation
N/A