Datadog tracing support

[Hartigan]

Hi,

This pull request allows to configure Datadog tracing for envoy proxy via CRDs.

[arkodg]

@Hartigan would be ideal if we can avoid a vendor specific field, can datadog tracing be supported via Otel ?

[Hartigan]

Hi @arkodg

I didn't add new specific field, I just add value to enum:

telemetry:
  tracing:
    samplingRate: 100
    provider:
      type: Datadog
      backendRefs:
      - name: datadog
        namespace: datadog
        port: 8126

For configuration it uses the same fields as opentelemetry. Not smth like https://gateway.envoyproxy.io/docs/api/extension_types/#zipkintracingprovider

I guess, to enable datadog tracing, we need to enable a specific plugin for envoy proxy. Therefore, there should be a way to describe the plugin name via yaml.

I made this request because envoy proxy has integration with datadog: https://www.envoyproxy.io/docs/envoy/latest/start/sandboxes/datadog-tracing

Ofc, it can be configured in another way with otel collector and specific exporter: https://docs.datadoghq.com/opentelemetry/collector_exporter/

I know one problem with solution via otel collector: datadog uses different headers for context propagation, and it's may be solved only via plugin/filter for envoy proxy.

Thanks

[arkodg]

this is probably the first feature that is vendor specific, and we dont have any precedence for this yet
@zhaohuabing @guydc and I discussed this yesterday and agreed that its okay to add a vendor specific feature if its already part of EnvoyProxy, looking for 👍 / 👎 from the rest of @envoyproxy/gateway-maintainers before we proceed with this PR

[Alice-Lilith]

Datadog is massive in the industry, we should make supporting it natively as easy as possible, especially if we already have people contributing the changes for us. If it weren't supported by Envoy Proxy and we had to write a custom service to add that native support, then I could understand being a little more on-the-fence about vendor specific inclusions in the API.

I think in general I do prefer APIs that are vendor-agnostic and support all of the options you need to configure whatever specific vendor you are using, but in instances like this where more specific handling is available then we should support it rather than trying to force people to use otel as a middle layer if that experience is not just as good and easy to use as native Datadog support.

[arkodg]

is CollectorHostname also needed ?

[Hartigan]

As I understand, CollectorHostname has default value which relates to CollectorCluster: https://github.com/envoyproxy/envoy/blob/b203e3e826af06e3639229f816f2a11245476728/api/envoy/config/trace/v3/datadog.proto#L40-L42. Also I didn't find similar fields for OpenTelemetry and Zipkin

[arkodg]

ok, have you been able to manually test this, and does it work ?

[Hartigan]

I see traces in Datadog APM from Envoy Proxy deployed with this gateway, and I found code which relates to this logic: https://github.com/envoyproxy/envoy/blob/39851cddac60be79f58ab33eafa35a1c9dda81a3/source/extensions/tracers/datadog/config.cc#L51-L58

Or I misunderstood the question and need somehow write a unit test for this

[arkodg]

great, thanks for checking, just wanted to make sure the integration actually works

[arkodg]

thanks @AliceProxy, looks like we have a majority now, removed the decision label

hey @Hartigan thanks for being patient, this PR should be ready for a review now, can you run make manifests && make generate and commit those changes ?
can you also add a test case in https://github.com/envoyproxy/gateway/tree/main/internal/xds/translator/testdata ?

[Hartigan]

@arkodg I will try to find time to add tests today/tomorrow

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.02%. Comparing base (1fbbe98) to head (ad59f5a).
Report is 50 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4298      +/-   ##
==========================================
+ Coverage   65.63%   66.02%   +0.39%     
==========================================
  Files         197      197              
  Lines       23540    23971     +431     
==========================================
+ Hits        15451    15828     +377     
- Misses       6979     7021      +42     
- Partials     1110     1122      +12     

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

[arkodg]

LGTM thanks !

[zirain]

/retest

[zirain]

it would be nice if you can add e2e and doc.

[zirain]

@Hartigan can you add e2e and doc for this?

[Hartigan]

Hi @zirain

Yes, I will try to find time on this week to create pull request with documentation and e2e tests

Thanks