Skip to content
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

improve OpenTelemetry config #34518

Closed
wants to merge 2 commits into from

Conversation

zeitlinger
Copy link

@zeitlinger zeitlinger commented Mar 8, 2023

  • configure OpenTelemetry via opentelemetry-sdk-extension-autoconfigure
  • use opentelemetry-micrometer-1.5 bridge
  • the change is not fully backwards compatible, which could be improving by having new auto config classes - should be discussed
  • all spring config options are still respected, both providing beans (e.g. for span filtering) and application properties
  • if the user does not configure settings explicitly in application properties, then the SDK auto-configuration is used, for example you can either use management.tracing.propagation or otel.propagators - not sure if this can be surprising
  • both sdk autoconfig and opentelemetry-micrometer-1.5 are alpha in otel, but haven't had any incompatible changes in the last months - I'll ask about it in the Java SIG

@pivotal-cla
Copy link

@zeitlinger Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

group("io.opentelemetry") {
imports = [
"opentelemetry-bom"
"opentelemetry-bom-alpha"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure a stable version of Boot should depend on something that is unstable.
Also, right now upgrading OTel to anything higher than 1.20.1 will break baggage propagation in Micrometer Tracing.


@Bean
@ConditionalOnMissingBean
public MeterRegistry openTelemetryMeterRegistry(OpenTelemetry openTelemetry, Clock clock) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be auto-configured by OpenTelemetry I might be mistaken but I think they do. Also, Micrometer has an OTLP registry so if you want an OTLP output, you can use that (its auto-configured).

@mhalbritter mhalbritter added the theme: observability Issues related to observability label Mar 9, 2023
@wilkinsona
Copy link
Member

Thanks for the proposal, @zeitlinger. Unfortunately, moving to an OpenTelemetry alpha is a non-starter. We'd also like to let other related changes settle before considering any further changes in this area. Those changes are coming in Micrometer, Micrometer Tracing and #34508 and should all be available in 3.1.0-M2 that is to be released later this month.

@wilkinsona wilkinsona closed this Mar 10, 2023
@wilkinsona wilkinsona added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 10, 2023
@zeitlinger zeitlinger deleted the otel_config branch February 2, 2024 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply theme: observability Issues related to observability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants