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

Separate out autoconfigure-spi artifact with tracing autoconfiguratio… #3570

Merged
merged 2 commits into from
Sep 1, 2021

Conversation

anuraaga
Copy link
Contributor

…n SPI.

I have not marked this artifact as alpha since I think it's ready to roll.

It means the same package is in both autoconfigure-spi and autoconfigure - this may cause JMS issues, but I'm not sure it's a big deal while metrics is alpha. If someone knows it is, I'd probably need to temporarily name the metrics SPIs package autoconfigure.spi.alpha

***! MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.extension.trace.propagation.B3ConfigurablePropagator (not serializable)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops - sorry I didn't run a full build on the previous PR

Copy link
Contributor

Choose a reason for hiding this comment

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

shame!

@jkwatson
Copy link
Contributor

I'm not an expert, but I thought the Java module system would blow up if two modules tried to export the same package and were being loaded by the same classloader, which these would, I think. :(

@jkwatson
Copy link
Contributor

Since we're breaking all of this, we could have autoconfigure.spi.traces and autoconfigure.spi.metrics to separate the packages. Thoughts?

@anuraaga
Copy link
Contributor Author

anuraaga commented Sep 1, 2021

Split up, it seems good and does solve the JMS problem.

Copy link
Contributor

@jkwatson jkwatson left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

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

In for a penny....

If we're breaking SPI let's make sure we do it right :)

@jkwatson jkwatson merged commit e8f0546 into open-telemetry:main Sep 1, 2021
jsuereth pushed a commit to jsuereth/opentelemetry-java that referenced this pull request Sep 2, 2021
open-telemetry#3570)

* Separate out autoconfigure-spi artifact with tracing autoconfiguration SPI.

* Split
This was referenced Dec 19, 2021
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