Skip to content

Conversation

axw
Copy link
Contributor

@axw axw commented Sep 15, 2025

Description

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.

Link to tracking issue

Part of #4970

Testing

Manually verified that service telemetry configuration still works with a collector binary.

Documentation

N/A

@axw axw force-pushed the service-settings-telemetryfactory branch 3 times, most recently from 902c8dc to 875114a Compare September 15, 2025 01:28
Copy link

codecov bot commented Sep 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.66%. Comparing base (55cad8b) to head (1c3d40e).
⚠️ Report is 3 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@axw axw force-pushed the service-settings-telemetryfactory branch from 875114a to 85d7247 Compare September 15, 2025 02:07
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`.
@axw axw force-pushed the service-settings-telemetryfactory branch from 85d7247 to 872b902 Compare September 15, 2025 02:41
@axw axw marked this pull request as ready for review September 15, 2025 03:23
@axw axw requested a review from a team as a code owner September 15, 2025 03:23
@axw axw requested a review from evan-bradley September 15, 2025 03:23
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking
Copy link
Contributor Author

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.

Copy link
Member

@bogdandrutu bogdandrutu left a 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)

@axw axw requested a review from bogdandrutu September 24, 2025 09:48
@bogdandrutu bogdandrutu dismissed their stale review September 25, 2025 04:09

Concerns fixe

Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants