[exporter/otlphttp] add custom profiles_endpoint config setting

[jackgopack4]

Description

Adds profiles_endpoint config setting for otlphttpexporter

Link to tracking issue

Updates #13504 (edit by @mx-psi)

Testing

unit and manual testing

Documentation

updated example config and README.md

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.72%. Comparing base (47efd15) to head (bbc0bf7).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13505      +/-   ##
==========================================
- Coverage   92.76%   92.72%   -0.04%     
==========================================
  Files         630      630              
  Lines       35515    35515              
==========================================
- Hits        32944    32933      -11     
- Misses       2025     2033       +8     
- Partials      546      549       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dmathieu]

Good catch, thank you.

[bogdandrutu]

The problem is that the config of this exporter is "stable" and we are adding possibly an unstable config here. What would be the path for this?

@open-telemetry/collector-approvers can you suggest?

[evan-bradley]

The problem is that the config of this exporter is "stable" and we are adding possibly an unstable config here. What would be the path for this?

Good point. I think we can proceed here for the following reasons:

• The module itself is still not considered stable: https://github.com/open-telemetry/opentelemetry-collector/blob/main/versions.yaml#L62
• The profiles signal is still listed as "in development": https://github.com/open-telemetry/opentelemetry-collector/blob/main/exporter/otlphttpexporter/metadata.yaml#L9
• Our versioning docs call our per-signal stability, allowing breaking changes for even 1.x components: https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/component-stability.md#versioning

We may want to clarify how stable/unstable config options at the same level like in this situation are treated. However everything I can see about our publicly-stated stability guarantees for the exporter seem to indicate that the exporter's handling of profiles and profile-related config options are still considered unstable.

[florianl]

With the v1development element in the endpoint, it clearly marks the endpoint as not stable. So I think, it is fine to go and I agree with #13505 (comment).

[mx-psi]

Our versioning docs call our per-signal stability, allowing breaking changes for even 1.x components: https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/component-stability.md#versioning

We don't allow breaking changes in the configuration (at least that was my intent when I wrote that part). In particular we say:

The versioning of a component, and the Go API stability guarantees that come with it, apply to ALL signals simultaneously, regardless of their stability level. This means that, once a component is marked as 1.x, signal-specific configuration options MUST NOT be removed or changed in a way that breaks our Go API compatibility promise, even if the signal is not stable.

I think for this particular option this means that we need to support this option forever if we merge this. I think this is fine since the profiling signal is here to stay, and the worst-case scenario is to keep this option working in some way if things change drastically.

[dmathieu]

As mentioned, it's extremely unlikely profiling would be dropped.
Furthermore, the expected breaking changes from profiling end up being in the OTLP protocol, not in how this interface is implemented.
So having this option for an unstable signal in a stable module seems fair to me.

[evan-bradley]

I think for this particular option this means that we need to support this option forever if we merge this.

The module itself is still 0.x, shouldn't we be okay to make API-only breaking changes for profiles before we make it 1.x?

[mx-psi]

I think for this particular option this means that we need to support this option forever if we merge this.

The module itself is still 0.x, shouldn't we be okay to make API-only breaking changes for profiles before we make it 1.x?

Yes, that is true, I guess the point Bogdan was trying to make is that we intend to mark this as 1.x at some point in the short/medium term. But anyway, as Damien pointed out, it is extremely unlikely that this would be changed in any way, so I am personally okay with taking the risk

[mx-psi]

@bogdandrutu Could you take a look at the discussion above and see if this unblocks things for you?

[mx-psi]

@bogdandrutu I think given the discussion above and the fact that this is still not 1.0 I am going to merge this and we can keep discussing on #13504 if you have further concerns. What do you think?

Per our conversation on Slack, dropping this since we have agreement from others this change is okay.

[bogdandrutu]

@mx-psi sounds good to me