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

Remove deprecated spring properties #10454

Merged

Conversation

zeitlinger
Copy link
Member

@zeitlinger zeitlinger commented Feb 9, 2024

Based on
- #10512
- #10408
- #10511
- #10530

Fixes ##10450

Bug fix

otel.propagators were not read correctly: https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/10454/files#diff-70b5a0d550786f16703fb5d2338924d7d213793f610bc24a640bd5f8fa64def5R81 -> split off to #10559

Breaking changes

All of these changes coverge towards the behavior of SDK auto config:

logging exporter

  • otel.exporter.logging.metrics.enabled -> otel.metrics.exporter=logging (same for traces, metrics)
  • otel.exporter.logging.enabled=true -> otel.metrics.exporter=logging, otel.traces.exporter=logging, otel.logs.exporter=logging

OTLP exporter

  • otel.exporter.otlp.metrics.enabled -> otel.metrics.exporter=otlp (same for traces, metrics) (enabled by default)
  • otel.exporter.otlp.enabled -> otel.metrics.exporter=otlp, otel.traces.exporter=otlp, otel.logs.exporter=otlp

Zipkin exporter

  • otel.exporter.zipkin.enabled -> otel.traces.exporter=zipkin

Propagation

  • otel.propagation.type=tracecontext,baggage -> otel.propagators=tracecontext,baggage

Resource

  • otel.springboot.resource.attributes -> otel.resource.attributes

@zeitlinger zeitlinger self-assigned this Feb 9, 2024
@zeitlinger zeitlinger requested a review from a team February 9, 2024 16:43
@github-actions github-actions bot requested a review from theletterf February 9, 2024 16:44
.gitignore Outdated Show resolved Hide resolved
@zeitlinger zeitlinger marked this pull request as draft February 10, 2024 06:55
@zeitlinger zeitlinger force-pushed the remove-deprecated-spring-properties branch 3 times, most recently from 0ad9d1b to 15a3ba2 Compare February 13, 2024 18:54
@zeitlinger zeitlinger marked this pull request as ready for review February 13, 2024 18:55
@@ -37,7 +37,7 @@ dependencies {
library("org.springframework.boot:spring-boot-starter-web:$springBootVersion")
library("org.springframework.boot:spring-boot-starter-webflux:$springBootVersion")

compileOnly("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi")
implementation("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure")
Copy link
Member

Choose a reason for hiding this comment

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

It should not be library("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure"), as the Spring dependencies? cc @laurit who should know

Copy link
Contributor

Choose a reason for hiding this comment

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

library means compileOnly + testImplementation. Using compileOnly is important in javaagent modules, which this module is not. It is also used to support the latest dep test where we change the version for library dependencies to latest version. In my opinion here implementation is the correct choice as it is a required dependency for this module.

@laurit
Copy link
Contributor

laurit commented Feb 14, 2024

@zeitlinger usually before removing the properties we do a deprecation cycle, was there a decision to skip this for these? In the pr description could you provide the list of removed properties and their replacements. @trask usually list breaking changes in the changelog, having these available here would make it easier for him.

@zeitlinger
Copy link
Member Author

@zeitlinger usually before removing the properties we do a deprecation cycle, was there a decision to skip this for these?

@trask and @jeanbisutti asked for the "fast track" because right now there are few users, but we want to make a push with the GA release

In the pr description could you provide the list of removed properties and their replacements. @trask usually list breaking changes in the changelog, having these available here would make it easier for him.

done 😄

@trask
Copy link
Member

trask commented Feb 15, 2024

will merge after the release (and sorry @zeitlinger looks like lots of conflicts from splitting out the bug fix)

@zeitlinger zeitlinger force-pushed the remove-deprecated-spring-properties branch from 10e9a67 to 574a7f1 Compare February 16, 2024 07:56
@zeitlinger zeitlinger force-pushed the remove-deprecated-spring-properties branch from 574a7f1 to 33a2c41 Compare February 16, 2024 08:47
@zeitlinger
Copy link
Member Author

will merge after the release (and sorry @zeitlinger looks like lots of conflicts from splitting out the bug fix)

I just had to ignore my all changes from the other PR 😄

@trask ready to merge now

@trask trask merged commit d8aa0f5 into open-telemetry:main Feb 16, 2024
47 checks passed
@zeitlinger zeitlinger deleted the remove-deprecated-spring-properties branch February 16, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants