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

Either the OTLP receiver stability or the configrpc and confighttp stability are wrong #7964

Closed
mx-psi opened this issue Jun 26, 2023 · 8 comments
Labels
area:config area:documentation bug Something isn't working discussion-needed Community discussion needed priority:p2 Medium receiver/otlp release:required-for-ga Must be resolved before GA release

Comments

@mx-psi
Copy link
Member

mx-psi commented Jun 26, 2023

We are discussing a general framework for defining stability of OpenTelemetry components on open-telemetry/oteps/pull/232.

As mentioned on open-telemetry/oteps#232 (comment), the OTLP traces & metrics receiver are currently marked as stable , while the configgrpc and confighttp modules (split in #7921) are still v0.X. This contradicts the rule laid out in the OTEP:

The maturity level for [...] component is, at most, the lowest level of its dependencies or priority components.

If we accept this rule it would imply one of the following:

  1. The OTLP traces & metrics receiver stability is wrong, since the configgrpc and confighttp modules are not v1.0
  2. The configgrpc and confighttp modules version is wrong, they should be v1.0
  3. The configgrpc and confighttp modules version is correct, but we provide stronger guarantees than semver does (in particular, we do not allow changes that would affect the stability of OTLP receiver as a Collector component, but may allow Go API only changes or changes in structs not used in the OTLP receiver configuration).

We should decide which one of the above is right and change the version/stability or document the stability guarantees. We could also argue that the rule in the OTEP is wrong.

If we are to follow the rule on the OTEP, (2) would also imply that configopaque, confignet, configauth (and therefore component) should be v1.0 since they are used by configgrpc and confighttp, which I think is not something we can do now (see #7954 (comment) for example).

@codeboten
Copy link
Contributor

I recall we had this discussion when talking about the stability level of components in contrib, and decided they could at best be as stable as the components in the collector they depend on. I think this should be the same for the components in core that depend on other components in core.

This would mean that the otlp exporter, otlp http exporter, and otlp receivers stability levels must be changed.

@mx-psi
Copy link
Member Author

mx-psi commented Jun 26, 2023

I recall we had this discussion when talking about the stability level of components in contrib, and decided they could at best be as stable as the components in the collector they depend on.

Yes, although it was ultimately dropped, we discussed adding such a rule here #3476 (comment)

This would mean that the otlp exporter, otlp http exporter, and otlp receivers stability levels must be changed.

I am okay with this, I think we should still try and be more careful with changing these in a backward incompatible way than with most other components, since they are so essential, but it seems like the way to go.

cc @bogdandrutu

@mx-psi
Copy link
Member Author

mx-psi commented Jun 27, 2023

The batch processor is also marked as stable. It does not depend on any configuration packages, so I think the case for reverting to beta is weaker there (I'd vote for keeping it stable), but this is a bigger rabbit hole than I initially expected...

@codeboten
Copy link
Contributor

codeboten commented Jun 27, 2023

It does depend on config/configtelemetry, but also on various other components that are still not stable.

@mx-psi
Copy link
Member Author

mx-psi commented Jun 29, 2023

If we consider any dependency to be problematic, then I think no component should be marked stable. I hate to reopen this discussion once again but specially if this ends up enshrined on an OTEP I think we should be consistent with the OTEP.

@bogdandrutu
Copy link
Member

I think there are difference between code stability and functionality stability which we need to also understand. v1.0 means API stability.

@jpkrohling
Copy link
Member

Under functionality stability, we should include not only the actual behavior, but also the interface with the end-user (the config), and the generated telemetry, especially metric names/dimensions, as users rely on those for alerts/dashboards.

The config aspect is why I think the confighttp not being stable should prevent otlpreceiver from being stable.

@atoulme atoulme added the release:required-for-ga Must be resolved before GA release label Dec 19, 2023
@codeboten
Copy link
Contributor

The plan for v1 has been published, we probably don't need to keep this issue opened anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:config area:documentation bug Something isn't working discussion-needed Community discussion needed priority:p2 Medium receiver/otlp release:required-for-ga Must be resolved before GA release
Projects
Archived in project
Development

No branches or pull requests

5 participants