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.

Sign up for GitHub

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

Jump to bottom

Manage opentelemetry-semconv-incubating #42186

Merged
merged 2 commits into from
Aug 1, 2024
Merged

Manage opentelemetry-semconv-incubating #42186

merged 2 commits into from
Aug 1, 2024

Conversation

ppalaga
Copy link
Contributor

@ppalaga ppalaga commented Jul 28, 2024

Needed to avoid convergence conflicts in Quarkus CXF - see

Error: Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.5.0:enforce (enforce) on project quarkus-cxf-integration-tracing-opentelemetry: 
Error:  Rule 1: org.apache.maven.enforcer.rules.dependency.DependencyConvergence failed with message:
Error:  Failed while enforcing releasability.
Error:  
Error:  Dependency convergence error for io.opentelemetry.semconv:opentelemetry-semconv-incubating:jar:1.26.0-alpha paths to dependency are:
Error:  +-io.quarkiverse.cxf:quarkus-cxf-integration-tracing-opentelemetry:jar:3.13.1-SNAPSHOT
Error:    +-org.apache.cxf:cxf-integration-tracing-opentelemetry:jar:4.0.5:compile
Error:      +-io.opentelemetry.semconv:opentelemetry-semconv-incubating:jar:1.26.0-alpha:compile
Error:  and
Error:  +-io.quarkiverse.cxf:quarkus-cxf-integration-tracing-opentelemetry:jar:3.13.1-SNAPSHOT
Error:    +-io.quarkus:quarkus-opentelemetry:jar:3.13.0:compile
Error:      +-io.opentelemetry.instrumentation:opentelemetry-instrumentation-annotations-support:jar:2.5.0-alpha:compile
Error:        +-io.opentelemetry.semconv:opentelemetry-semconv-incubating:jar:1.25.0-alpha:compile
Error:  -> [Help 1]

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/tracing labels Jul 28, 2024
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 28, 2024

/cc @brunobat (opentelemetry), @radcortez (opentelemetry)

@ppalaga
Copy link
Contributor Author

ppalaga commented Jul 28, 2024

Would you like to review @brunobat ?

@ppalaga ppalaga mentioned this pull request Jul 28, 2024
Merged
1 task
ppalaga added a commit to quarkiverse/quarkus-cxf that referenced this pull request Jul 28, 2024
@ppalaga
Manage opentelemetry-semconv-incubating temporarily until quarkusio/q…
9b01f85 
…uarkus#42186 reaches us
@quarkus-bot

This comment has been minimized.

Sign in to view
@radcortez
Copy link
Member

Integration tests OTel failures seem suspicious. Rerunning...

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 29, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

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

✅ 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

📦 integration-tests/opentelemetry

✖ io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.testOTelInjections - History

  • Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.reset(OpenTelemetryInjectionsTest.java:26)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)

⚙️ JVM Tests - JDK 21

📦 integration-tests/opentelemetry

✖ io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.testOTelInjections - History

  • Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.reset(OpenTelemetryInjectionsTest.java:26)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

@ppalaga
Copy link
Contributor Author

ppalaga commented Jul 29, 2024

Integration tests OTel failures seem suspicious. Rerunning...

Develocity marks them as flaky, does it not?

ppalaga added a commit to quarkiverse/quarkus-cxf that referenced this pull request Jul 30, 2024
@ppalaga
Manage opentelemetry-semconv-incubating temporarily until quarkusio/q…
3312f1e 
…uarkus#42186 reaches us
ppalaga added a commit to quarkiverse/quarkus-cxf that referenced this pull request Jul 30, 2024
@ppalaga
Manage opentelemetry-semconv-incubating temporarily until quarkusio/q…
a3049fb 
…uarkus#42186 reaches us
@radcortez
Copy link
Member

Yes, but I just find it suspicious that the test failed for both 17 and 21, but I did see the same symptoms in another PR. I'll have a look.

gsmet
gsmet reviewed Jul 31, 2024
View reviewed changes
bom/application/pom.xml
Comment on lines -6446 to -6455
<exclusions>
<exclusion>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-bom</artifactId>
</exclusion>
<exclusion>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-api</artifactId>
</exclusion>
</exclusions>
Copy link
Member

Choose a reason for hiding this comment

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

With this, you do a bit more than adding something to the BOM as advertised in the title. Could you clarify?

I'm all for what you added below but not sure I would be comfortable merging the part above without having @brunobat checking as it's something that has been added explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

opentelemetry-semconv depends on neither opentelemetry-bom nor opentelemetry-api so the exclusions are completely superfluous. Perhaps they were dependencies in the past. The change is in a separate commit.

gsmet
gsmet approved these changes Aug 1, 2024
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

OK, thanks for the clarification, it makes perfect sense.

@gsmet gsmet merged commit a38d559 into quarkusio:main Aug 1, 2024
53 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.14 - main milestone Aug 1, 2024
@gsmet gsmet modified the milestones: 3.14 - main, 3.13.1 Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsmet gsmet gsmet approved these changes

@radcortez radcortez radcortez approved these changes

Assignees
No one assigned
Labels
area/dependencies Pull requests that update a dependency file area/tracing triage/flaky-test
Projects
None yet
Milestone
3.13.1
Development

Successfully merging this pull request may close these issues.
3 participants
@ppalaga @radcortez @gsmet