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

Namespace not set when sending *directly* to Datadog #1162

Open
Tracked by #2878
krisztian-sala opened this issue May 26, 2022 · 19 comments
Open
Tracked by #2878

Namespace not set when sending *directly* to Datadog #1162

krisztian-sala opened this issue May 26, 2022 · 19 comments
Assignees
Labels
component/open-telemetry OTLP, Datadog, Prometheus, etc. and the integrations around it. raised by user

Comments

@krisztian-sala
Copy link

I have set the Datadog namespace both with the DD_ENV environment variable (which works for other services) and also from the router configuration like so:

...
      tracing:
        trace_config:
          service_name: "${DD_SERVICE:apollo-router}"
          service_namespace: "${DD_ENV:my-namespace}"

But the namespace is still set as none

How can I set it correctly?

@garypen
Copy link
Contributor

garypen commented May 26, 2022

You router configuration is correct. In your various config elements your service_namespace will take the value of ${DD_ENV} or, if not set, "my-namespace".

I modified the source and made a debug build of the router to print out the config used to build the router trace information and confirmed it was setting service_namespace:

2022-05-26T12:46:45.100927Z  INFO apollo_router::plugins::telemetry::config: trace_config: Config { sampler: ParentBased(AlwaysOn), id_generator: IdGenerator { _private: () }, span_limits: SpanLimits { max_events_per_span: 128, max_attributes_per_span: 128, max_links_per_span: 128, max_attributes_per_event: 128, max_attributes_per_link: 128 }, resource: Some(Resource { attrs: {Key("process.executable.name"): String("tester"), Key("service.name"): String("apollo-router"), Key("service.namespace"): String("my-namespace")} }) }

(If you scroll right far enough above, you can see service_namespace is set to "my-namespace".)

I'm curious how you know the service_name is set to none in your case. Are you running your own build?

@garypen garypen self-assigned this May 26, 2022
@krisztian-sala
Copy link
Author

I think I explained it wrong. So I don't actually know what the service_namespace variable's value is, I'm using the v0.9.2 version, so no custom build.
The router is running in Kubernetes and what I'm trying to achieve is to set the Kubernetes namespace that the router is part of as the env variable in Datadog.

Currently, whatever I do, the env is none
image

I thought that the service_namespace sets that value. But I also tried by setting a custom trace attribute like so:

telemetry:
  tracing:
    trace_config:
      service_name: "${DD_SERVICE:apollo-router}"
      service_namespace: "${DD_ENV:my-namespace}"
      sampler: 1
      attributes: 
        env: "${DD_ENV:my-namespace}"

@bnjjj
Copy link
Contributor

bnjjj commented May 27, 2022

Apparently we're using service_namespace and pass it to attributes named with service.namespace (https://github.com/apollographql/router/blob/main/apollo-router/src/plugins/telemetry/config.rs#L224) . Therefore on your traces you should have an attribute named service.namespace with the value you're looking for. Let me know if it helps

@krisztian-sala
Copy link
Author

Unfortunately I don't see that attribute or any other that I set in telemetry.tracing.trace_config.attributes appear in my trace. The only attributes that show up in Datadog are these:

Screenshot 2022-05-30 at 11-51-17 APM Traces Datadog

@krisztian-sala
Copy link
Author

Any news about this? Not having proper monitoring is a blocker for us to go to production with the router.

It doesn't send any custom attribute to Datadog. But the most important would be the env, which is currently none in all cases.

@BrynCooke
Copy link
Contributor

This is something that we need to implement.
The existing datadog opentelemetry implementation is only understands service.name out of the box.

It doesn't help that what is currently called attributes in our config yaml should probably be called resource as it pertains to this: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/semantic_conventions/README.md

To move forward on this I suggest the following:

telemetry:
  tracing:
    common:
      attributes:
        static:
          - name: "version"
            value: "v1.0.0"
        request:
          - header:
               named: "content-type"
               rename: "payload_type"
               default: "application/json"
          - header:
              named: "x-custom-header-to-add"
        response:
          - body:
              path: errors.extensions.status
              name: extended_status
              default: optional_default_value
        context:
          - named: "foo"
            default: "application/json"

@bnjjj If we can come to an agreement on yaml format that sits nicely for both metrics and tracing then we can probably make this happen quickly. Let's discuss.

@BrynCooke
Copy link
Contributor

We've settled on the following which should be consistent with the rest of our config:

telemetry:
  tracing: 
    common:
      resource:      
      attributes:
        router:
          static:
            - name: "version"
              value: "v1.0.0"
          request:
            - header:
                named: "content-type"
                rename: "payload_type"
                default: "application/json"
            - header:
                named: "x-custom-header-to-add"
          response:
            - body:
                path: errors.extensions.status
                name: extended_status
                default: optional_default_value
          context:
            - named: "foo"
          
        subgraph:
          all:
            static:
              - name: "version"
                value: "v1.0.0"
              request:
                - header:
                    named: "content-type"
                    rename: "payload_type"
                    default: "application/json"
                - header:
                    named: "x-custom-header-to-add"
              response:
                - body:
                    path: errors.extensions.status
                    name: extended_status
                    default: optional_default_value
              context:
                - named: "foo"
          subgraphs:
            products:
              static:
                - name: "version"
                  value: "v1.0.0"
                request:
                  - header:
                      named: "content-type"
                      rename: "payload_type"
                      default: "application/json"
                  - header:
                      named: "x-custom-header-to-add"
                response:
                  - body:
                      path: errors.extensions.status
                      name: extended_status
                      default: optional_default_value
                context:
                  - named: "foo"

@bnjjj
Copy link
Contributor

bnjjj commented Jun 16, 2022

related to #1270

@bnjjj bnjjj assigned bnjjj and unassigned garypen Jun 29, 2022
@bnjjj bnjjj removed the triage label Jun 29, 2022
@abernix abernix changed the title Datadog namespace not set Namespace not set when sending directly to Datadog Aug 12, 2022
@abernix abernix changed the title Namespace not set when sending directly to Datadog Namespace not set when sending *directly* to Datadog Aug 12, 2022
@bnjjj
Copy link
Contributor

bnjjj commented Sep 12, 2022

This PR is currently blocked by open-telemetry/opentelemetry-rust#876

@abernix
Copy link
Member

abernix commented Oct 25, 2022

@bnjjj Is this expected to be fixed via #1948?

@bnjjj
Copy link
Contributor

bnjjj commented Oct 25, 2022

Unfortunately no, we need a new release (0.19.0 or 0.18.1)

@krisztiansala
Copy link

Any news on this?

@BrynCooke
Copy link
Contributor

BrynCooke commented Jan 3, 2023

@krisztiansala Unfortunately we are still waiting on an Otel release.

Are you using a custom router build? If so then you can use the latest router and use the following patch to have this working:

[patch.crates-io]
 opentelemetry = { git = "https://github.com/open-telemetry/opentelemetry-rust.git", rev = "e5ef3552efab2bdbf2f838023c37461cd799ab2c"}
 opentelemetry-http = { git = "https://github.com/open-telemetry/opentelemetry-rust.git", rev = "e5ef3552efab2bdbf2f838023c37461cd799ab2c"}
 opentelemetry-jaeger = { git = "https://github.com/open-telemetry/opentelemetry-rust.git", rev = "e5ef3552efab2bdbf2f838023c37461cd799ab2c"}

We'll also look at doing something for our binary builds shortly.

@krisztiansala
Copy link

I've tried this, but it didn't work for me - everything is still the same. I will wait for the release, maybe it will be different.

@abernix
Copy link
Member

abernix commented Mar 17, 2023

This is now waiting on open-telemetry/opentelemetry-rust#965, which we're hoping comes soon. (Suggested to be this weekend!)

@abernix
Copy link
Member

abernix commented Mar 29, 2023

The fix for this is purportedly is now in https://github.com/open-telemetry/opentelemetry-rust/releases/tag/v0.19.0. I'll see about getting the integration of that upgrade prioritized on our side.

@abernix
Copy link
Member

abernix commented May 15, 2023

As an update, we are still waiting on an upstream dependency (one which depends on opentelemetry-rust, itself) to actually move this along — tokio-rs/tracing-opentelemetry#9.

@garypen
Copy link
Contributor

garypen commented Jun 5, 2023

I spent a little time investigating this as part of the update to opentelemetry 0.19.0. There isn't a specific method for capturing service_namespace() in the opentelemetry-datadog library. I tried adding some code to manually copy the setting has an attribute and this didn't work.

The only workaround that I could concoct was to explicitly add configuration for service_namespace as an attribute in trace_config, which is clearly undesirable. We'll have to leave this open for now and return to it after 0.19.0 is merged.

@abernix abernix added the component/open-telemetry OTLP, Datadog, Prometheus, etc. and the integrations around it. label Aug 21, 2023
@BrynCooke BrynCooke assigned BrynCooke and unassigned bnjjj Oct 10, 2023
@BrynCooke
Copy link
Contributor

Let's submit a PR to otel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/open-telemetry OTLP, Datadog, Prometheus, etc. and the integrations around it. raised by user
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants