Enable propagation of the trace context from collector

[pavankrish123]

trafficstars

Description:

Added support to propagate traceparent etc headers from collector to the downstream entities that process the export requests from collector. The feature uses newly released autoprop package from go contrib .

The feature is behind the gate telemetry.allowTraceContextPropagation

Link to tracking Issue: 5572

Testing:

running

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

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] Traceparent:[00-62f74abcde1f2ca18f0bb7faa8ba385a-9cc603f55b09d95f-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:
  pipelines:
    traces:
      receivers: [otlp]
      exporters: [otlphttp, logging]
      processors: [batch]

[codecov[bot]]

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%)	:arrow_up:
service/collector.go	78.90% <0.00%> (+2.34%)	:arrow_up:
service/service.go	68.42% <0.00%> (+3.15%)	:arrow_up:
pdata/internal/metrics.go	92.91% <0.00%> (+6.78%)	:arrow_up:

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

[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. https://github.com/open-telemetry/opentelemetry-collector/issues/5572

[pavankrish123]

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

[jpkrohling]

As a rule of thumb in the collector we are moving away from env variables in general

While I agree with the general feeling, this goes into a bigger standard: OpenTelemetry SDK should generally be configurable via env vars.

[bogdandrutu]

OpenTelemetry SDK should generally be configurable via env vars.

Unfortunately this is true because the TC members were lazy (starting with the person who writes this) and have not put the effort into review/propose a configuration (yaml/json) and lazily accepted env vars as the only way. /cc @jsuereth

[pavankrish123]

Hi @bogdandrutu, here are our options I can keep the PR pending till we will have feature available in OTEL go SDK's autoprop module to set the propagators via configuration. @Aneurysm9 mentioned it is in works. Reference to the issue I opened https://github.com/open-telemetry/opentelemetry-go-contrib/issues/2590

cc: @jpkrohling

Thanks , Pavan

[pavankrish123]

@bogdandrutu, I have updated PR per your suggestion using the new API added to autoprop module in otel-go-contrib. The API is not released yet, so I will keep the PR in draft. But please take a look into the new commit and let me know if you think I am heading in the right direction, I am adding unit tests as well.

Latest tests -

running

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

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] Traceparent:[00-62f74abcde1f2ca18f0bb7faa8ba385a-9cc603f55b09d95f-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:
  pipelines:
    traces:
      receivers: [otlp]
      exporters: [otlphttp, logging]
      processors: [batch]

[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.

[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