Enable propagation of the trace context from collector

[pavankrish123]

Description:

Added support to propagate traceparent etc headers from collector to the downstream entities that process the export requests from collector.

Link to tracking Issue: 5572

Testing:

running

./otelcorecol_darwin_amd64 --config test.yaml 

gives

2022/07/27 21:59:24 request headers: map[Accept-Encoding:[gzip] B3:[e027c66374cdfd8c54704a92cd7759eb-7ce684e76639b14e-0] Content-Encoding:[gzip] Content-Length:[860] Content-Type:[application/x-protobuf] Traceparent:[00-e027c66374cdfd8c54704a92cd7759eb-7ce684e76639b14e-00] User-Agent:[Local OpenTelemetry Collector binary, testing only./0.55.0-dev (darwin/amd64)]]

for config

receivers:
  otlp:
    protocols:
      grpc:

exporters:
  otlphttp:
    endpoint: http://0.0.0.0:3000
    traces_endpoint: http://0.0.0.0:3000/v1/trace
    tls:
      insecure: true

  logging:


processors:
  batch:

service:
  telemetry:
    traces:
      propagators:
        - tracecontext
        - b3
  pipelines:
    traces:
      receivers: [otlp]
      exporters: [otlphttp, logging]
      processors: [batch]

(and)

2022/07/28 00:14:57 request headers: map[Accept-Encoding:[gzip] Content-Encoding:[gzip] Content-Length:[859] Content-Type:[application/x-protobuf] User-Agent:[Local OpenTelemetry Collector binary, testing only./0.55.0-dev (darwin/amd64)]]

for config

receivers:
  otlp:
    protocols:
      grpc:

exporters:
  otlphttp:
    endpoint: http://0.0.0.0:3000
    traces_endpoint: http://0.0.0.0:3000/v1/trace
    tls:
      insecure: true

  logging:


processors:
  batch:

service:
  pipelines:
    traces:
      receivers: [otlp]
      exporters: [otlphttp, logging]
      processors: [batch]

[codecov]

Codecov Report

Merging #5670 (b9272d5) into main (d1b46b7) will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5670      +/-   ##
==========================================
+ Coverage   91.73%   91.86%   +0.13%     
==========================================
  Files         197      197              
  Lines       12465    12472       +7     
==========================================
+ Hits        11435    11458      +23     
+ Misses        814      800      -14     
+ Partials      216      214       -2     

Impacted Files	Coverage Δ
service/telemetry.go	90.38% <100.00%> (+1.17%)	⬆️
service/collector.go	78.90% <0.00%> (+2.34%)	⬆️
service/service.go	68.42% <0.00%> (+3.15%)	⬆️
pdata/internal/metrics.go	92.91% <0.00%> (+6.78%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[mx-psi]

👋 I think you should open an issue to discuss the use case for this before we decide on this PR.

I also left some suggestions to make the configuration more consistent with existing practices on the Collector.

[pavankrish123]

👋 I think you should open an issue to discuss the use case for this before we decide on this PR.

@mx-psi the linked issue is in the PR description. #5572

[mx-psi]

The config struct looks sensible, and it sounds like a feature we can support at this level.

I don't feel very comfortable with comitting to autoprop which has had a single version so far and is not 1.0, but I guess since we can still make breaking changes we can change our mind later (specially if we have this off-by-default for now)

[pavankrish123]

@Aneurysm9 adjusted the PR per your suggestion on using feature gate. Please let me know WDYT. PR description modified accordingly.

[pavankrish123]

Hello @bogdandrutu @jpkrohling - The autoprop api is officially released and I have updated the PR. The PR is complete. Please review and give your blessings. :)

service:
  telemetry:
    traces:
      propagators: "tracecontext,b3"

• The feature is available as a collector configuration now per @bogdandrutu suggestion and it is behind a feature gate.
• I will work on getting docs added under https://opentelemetry.io/docs/collector once the PR is merged. There is no documentation as such in this context (collectors self telemetry) - So will have to write one from scratch.

[jpkrohling]

Please, revert the "retract" changes to the go.mod files. If they were intentional, please open a new PR with those isolated changes.

Other than that, LGTM, but I believe @bogdandrutu needs to sign off as well, given he had comments before.

[pavankrish123]

Please, revert the "retract" changes to the go.mod files. If they were intentional, please open a new PR with those isolated changes.

Other than that, LGTM, but I believe @bogdandrutu needs to sign off as well, given he had comments before.

done!

[pavankrish123]

As a rule of thumb in the collector we are moving away from env variables in general (allow them only as part of the yaml config via embedding). So I think we should offer an yaml alternative first for this feature.

(Done!)

@bogdandrutu can you please review and let me know.

[pavankrish123]

Updated the PR per @bogdandrutu suggestion, the configuration is a list now

receivers:
  otlp:
    protocols:
      grpc:

exporters:
  otlphttp:
    endpoint: http://0.0.0.0:3000
    traces_endpoint: http://0.0.0.0:3000/v1/trace
    tls:
      insecure: true

  logging:


processors:
  batch:

service:
  telemetry:
    traces:
      propagators:
        - tracecontext
        - b3
  pipelines:
    traces:
      receivers: [otlp]
      exporters: [otlphttp, logging]
      processors: [batch]

./otelcorecol_darwin_amd64 --config test.yaml --feature-gates=telemetry.allowTraceContextPropagation

produces the following headers in the export request.

2022/08/12 11:13:30 request headers: map[Accept-Encoding:[gzip] B3:[d6550dfa8b9c5e0f10acd6533391f08f-4bd7dc181ad45baf-0] Content-Encoding:[gzip] Content-Length:[276] Content-Type:[application/x-protobuf] Traceparent:[00-d6550dfa8b9c5e0f10acd6533391f08f-4bd7dc181ad45baf-00] User-Agent:[Local OpenTelemetry Collector binary, testing only./0.58.0-dev (darwin/amd64)]]

[pavankrish123]

@bogdandrutu - just summarizing the suggestions from you, to make sure I didn;t miss any

1. Limiting the propagators that we allow (tracecontext, b3) - this can be done if we don't use autoprop module at all(which pulls all the propgators)
2. Changing the default behavior of the feature gate
   1. when feature gate flag is passed and there is no configuration in collectors yaml, we enable tracecontext by default
   2. when feature gate flag is not passed and there is no configration in collectors yaml, we don;t propagate context at all.
   3. when telemetry.traces configuration is present, we ignore the feature gate flag and honor the configuration and propagate the context

Please let me know if my understanding of these changes you suggested is correct?

[bogdandrutu]

Limiting the propagators that we allow (tracecontext, b3) - this can be done if we don't use autoprop module at all(which pulls all the propgators)

Correct

Changing the default behavior of the feature gate
when feature gate flag is passed and there is no configuration in collectors yaml, we enable tracecontext by default
when feature gate flag is not passed and there is no configration in collectors yaml, we don;t propagate context at all.
when telemetry.traces configuration is present, we ignore the feature gate flag and honor the configuration and propagate the context

This is too complicated, let's go with a simple thing. Remove the featuregate and make default config to be "empty list" which means this is disable by default. Then in a separate PR we can discuss about adding a featuregate to change the default config.

[pavankrish123]

Thanks @bogdandrutu - updated the PR per your suggestions. Kindly review