open-telemetry / opentelemetry-java-contrib

https://opentelemetry.io
Apache License 2.0
158 stars 126 forks source link

Inline incubating attributes + central semconv-incubating dependency #1298

Closed SylvainJuge closed 4 months ago

SylvainJuge commented 5 months ago

Initial intent:

The non-incubating semconv dependency is transitively provided by the opentelemetry-instrumentation-bom-alpha BOM, which unfortunately does not include the semconv-incubating artifact.

As a possible follow-up improvement, we could include it in the instrumentation BOM as it should align with the version of semconv by default, also it would help the maintainers of distributions by allowing to directly pull both the semconv and semconv-incubating dependencies from the BOM instead of having to keep two versions in sync.


Updated description:

SylvainJuge commented 4 months ago

@jack-berg I now understand that having direct dependencies on the incubating attributes artifact can be problematic and why we'd like to avoid direct references to them in the published artifacts.

However, for the developer/maintainer in this repository there are definitely some benefits to have it, and I don't think that manually duplicating the incubating attributes is something we can enforce as it makes later maintenance more tedious: whenever new attributes are promoted/modified in a new semconv version, you have to grep the code to modify and update it, whereas with direct dependencies we can get automatic upgrades or compile time errors that are easier to manage.

Now that we have a separate artifact for incubating dependencies, we could maybe shade it in every sub-project of this repo where it's used, so it would make any artifact depending on it effectively immune to be used with a different incubating attributes artifact in the classpath.

SylvainJuge commented 4 months ago

After failing to find an "easier" solution with the annotation processor (https://github.com/open-telemetry/opentelemetry-java-contrib/issues/1322) I think that duplicating the incubating attributes and testing with the semconv-incubating artifact is the best option for now.

I have now updated this PR to implement this strategy for all the modules in contrib.