Move required OpenTelemetry packages to prometheus/prometheus about prometheus HOT 10 CLOSED

👋

• It doesn't actually help at all to move it to a third, neutral repo because we still have cyclical dependencies.
• collector feature gates don't make sense in a "neutral" package

I brought this topic up with the team and the above two things are the main ones that make me think it's best to have different code bases.

• Having a separate neutral package is a bit clunky and it would also not solve the cyclical dependency per-se it would still require refactoring the use of PRW protos.
• As mentioned the collector feature gates don't make sense in Prometheus which makes sharing the code base less ideal.
• The prometheus remote write exporter code exists to do the otlp to prw translation but the code that will now live in prometheus could know more about Prometheus internals to try to make the native ingestion faster. So this divergence with less guarantees might be helpful down the road.

Although this means divergence we're still very involved in the Prometheus - Opentelemetry calls and if we make any meaningful improvements over time we'll bring them up 💪 cc @dashpole

Sometime soon @aknuds1 and I plan to work on a PR to make this a reality and finally close this issue.

from prometheus.

Here is the first improvement to PRW translation :)

https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/32605/files#diff-57a6ba7af4a99bae6ac52e8adb5c974559e0bcd46ce597488cd0031facb32b89

from prometheus.

I'll bring this topic to today's prometheus - otel workgroup to see if we can expedite owning the lib now that its been months of updating it, having the endpoint live in our release etc

Knowing that this is causing issues to some users is even more reason to pursue it and then we'll be more in control to make the changes we need.

from prometheus.

Reading back through, the primary concerns with the move were:

1. Dependency tree: OTel contrib -> Prometheus -> OTel core dependency will make breaking changes to OTel core modules extremely difficult to handle updates for.
2. Stability guarantees. OpenTelemetry components would potentially have breaking changes pushed on them via dependency updates, which are harder to review.

The dependency tree for both translator packages currently contains these collector dependencies:

• go.opentelemetry.io/collector/pdata @ v1.4.1
• go.opentelemetry.io/collector/semconv @ v0.97.1
• go.opentelemetry.io/collector/featuregate @ v1.4.1
• github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal @ v0.97.0

Semconv is used for AttributeServiceName, AttributeServiceInstanceID, and AttributeServiceNamespace constants. Those could be copied, or we could use constants from opentelemetry-go.

coreinternal is used for test data generation, and could be copied.

Given other dependencies are stable, I think it eliminates (1) as a concern.

For Stability guarantees, we had discussed these as potential parts of the agreement:

• The package will follow the OTel --> Prometheus specification
• The package will release with 0.* versioning, but will not make breaking changes to the Go API it offers except for breaking changes to the protobuf it inherits from prometheus/prometheus/prompb.
• The package will still be owned by members of the OpenTelemetry Prometheus-wg to start. Proposed: @dashpole and @Aneurysm9. Over time, we expect members of the prometheus to take a more active role in maintaining the package.
• If changes to the package break collector functionality and block prometheus dependency updates, the Prometheus community will help with any needed rollbacks, cherrypicks, or patch releases needed to address them.
• [New] Significant behavior changes will be gated by collector feature gates.

from prometheus.

@jesusvazquez WDYT of this problem? You have more history on OTLP in Prometheus than I do.

In particular, I'd like to understand if there are talks about moving the storage/remote/otlptranslator/ directory contents into Prometheus and out of opentelemetry-collector-contrib (so the latter depends on the former).

from prometheus.

I don't have opinion on moving packages, I recall one discussion if owners of OTel contrib would like to "donate" this lib to us and how maintainership would work for it etc. I am sure @gouthamve and @dashpole know more.

In the mean time, since we don't expect people to import or use this library we vendor in Otel collector or SDKs, why not removing code that uses any global variables in our otlptranslator?

from prometheus.

Original thread: https://cloud-native.slack.com/archives/C01LSCJBXDZ/p1689161965395529

Discussion from the prom-wg yesterday: https://docs.google.com/document/d/19bnXziPn2MZ9wO6684UoI4D-LCjGL5bTJkGhux29bx8/edit#bookmark=id.399s77tz4cjr

from prometheus.

Notes from today's WG meeting:

• It doesn't actually help at all to move it to a third, neutral repo because we still have cyclical dependencies.
• collector feature gates don't make sense in a "neutral" package
• Prometheus may not want to translate to the proto format anyways. It is convenient, since it can then re-use PRW ingestion support, but it may want more direct translation or storage. So maybe it makes sense to diverge.
• If we don't use the PRW protos directly in the translation package, there isn't a cyclical dependency for Prometheus anymore.
• Prometheus is happy to allow OTel folks to be maintainers, even if the package is prometheus/prometheus

from prometheus.

@dashpole @Aneurysm9 I'm trying to diverge code while respecting the licensing in this PR #13991

Please have a look mostly at the file header where the relicensing is happening.

from prometheus.

This is now completed and this issue can be closed. Thanks everyone for the help 💪

from prometheus.