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

Bump OTel 1.39. and instrumentation to 2.5.0 #41521

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

brunobat
Copy link
Contributor

@brunobat brunobat commented Jun 27, 2024

BREAKING CHANGES.

This updates the opentelemetry-java bom from 1.32 to 1.39 and the opentelemetry-instrumentation alpha bom from 1.32 to 2.50.

The breaking changes for the users are:

  • HTTP semantic conventions have changed. See the full list of changes;
  • quarkus.otel.semconv-stability.opt-in system property was removed because users cannot opt-in anymore;
  • DB Span names have changed in the case of table creation;
  • Deprecated annotation for io.opentelemetry.extension.annotations.WithSpan has been removed. Please use the new io.opentelemetry.instrumentation.annotations.WithSpan, as previously announced.

For later:

Other things to look at:

  • Removed obsolete tests for the now removed @WithSpan legacy annotation.
  • Removed the triple test executions depending on the value of the quarkus.otel.semconv-stability.opt-in system property. This will speed up tests considerably.
  • Documentation updates.

For more details please check:
https://github.com/open-telemetry/opentelemetry-java/blob/main/CHANGELOG.md
https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/CHANGELOG.md

@brunobat
Copy link
Contributor Author

brunobat commented Jun 27, 2024

Will squash the commits after the PR is reviewed as it makes it easy to spot change implications.

@brunobat
Copy link
Contributor Author

This will depend on SmallRye reactive messaging changes being currently made... Will post PR here, when available.

@brunobat brunobat requested a review from radcortez June 27, 2024 16:59

This comment has been minimized.

Copy link

github-actions bot commented Jun 27, 2024

🎊 PR Preview 72af1d9 has been successfully built and deployed to https://quarkus-pr-main-41521-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

Copy link
Member

@radcortez radcortez left a comment

Choose a reason for hiding this comment

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

At least we got rid of a bunch of code :)

Copy link

@trask trask left a comment

Choose a reason for hiding this comment

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

❤️ sorry you have had to deal with all of the changes in these unstable components. it looks like most of the remaining unstable dependencies are related to DB, RPC and messaging semconv. I think DB and messaging semconv will be stable by end of the year, and we will start working on RPC semconv stability in the next few months. In the opentelemetry-java-instrumentation repo we are currently planning to bump from 2.x to 3.x once we are ready to switch over to stable semconv for all three (DB, RPC and messaging) by default. I noticed that you followed the opt-in flag for http which is awesome, but I would definitely understand if you didn't go through that for future updates since it's quite painful unless you are getting pushback on this from your users (in general we haven't had too much expectation of libraries emitting native instrumentation before the related semconv is stabilized).

@@ -104,7 +100,7 @@
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-api-events</artifactId>
<artifactId>opentelemetry-api-incubator</artifactId>
Copy link

Choose a reason for hiding this comment

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

what do you use this module for?

Copy link
Contributor Author

@brunobat brunobat Jun 28, 2024

Choose a reason for hiding this comment

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

We need to reset for tests the GlobalEventLoggerProvider otherwise it throws an error when we execute dev mode. I remember that native would also failed without it.
It's usage comes from the SDK Autoconfiguration.

@gsmet
Copy link
Member

gsmet commented Jul 1, 2024

@brunobat I let you squash and merge when you're ready.

@brunobat
Copy link
Contributor Author

brunobat commented Jul 2, 2024

I think I will hold until metrics is merged.

@brunobat
Copy link
Contributor Author

brunobat commented Jul 3, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link

quarkus-bot bot commented Jul 12, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 59cdeca.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

tcks/pom.xml Show resolved Hide resolved

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

@ozangunalp ozangunalp left a comment

Choose a reason for hiding this comment

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

Needs rebasing for version change

Copy link

quarkus-bot bot commented Jul 15, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit b6f5499.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/smallrye-reactive-messaging/deployment

io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector - History

  • Expecting actual: ["-4","-5","-6","-7","-8","-9","-10","-11"] to start with: ["-3", "-4", "-5", "-6"] - java.lang.AssertionError
java.lang.AssertionError: 

Expecting actual:
  ["-4","-5","-6","-7","-8","-9","-10","-11"]
to start with:
  ["-3", "-4", "-5", "-6"]

	at io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector(ConnectorChangeTest.java:36)

📦 integration-tests/kotlin

io.quarkus.kotlin.maven.it.KotlinRemoteDevModeIT.testThatTheApplicationIsReloadedOnKotlinChange - History

  • org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.maven.it.RunAndCheckWithAgentMojoTestBase was not fulfilled within 1 minutes. - java.lang.RuntimeException
java.lang.RuntimeException: org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.maven.it.RunAndCheckWithAgentMojoTestBase was not fulfilled within 1 minutes.
	at io.quarkus.maven.it.RunAndCheckWithAgentMojoTestBase.runAndCheck(RunAndCheckWithAgentMojoTestBase.java:86)
	at io.quarkus.kotlin.maven.it.KotlinRemoteDevModeIT.testThatTheApplicationIsReloadedOnKotlinChange(KotlinRemoteDevModeIT.java:23)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.maven.it.RunAndCheckWithAgentMojoTestBase was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)

@brunobat brunobat merged commit 59dcb16 into quarkusio:main Jul 15, 2024
52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.13 - main milestone Jul 15, 2024
jedla97 added a commit to jedla97/quarkus-test-suite that referenced this pull request Jul 16, 2024
@gsmet gsmet changed the title Bump OTel 1.39. and instrumentation no 2.5.0 Bump OTel 1.39. and instrumentation to 2.5.0 Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

6 participants