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

Add configuration for trace_id #2080

Closed
bnjjj opened this issue Nov 9, 2022 · 2 comments · Fixed by #2131
Closed

Add configuration for trace_id #2080

bnjjj opened this issue Nov 9, 2022 · 2 comments · Fixed by #2131
Assignees

Comments

@bnjjj
Copy link
Contributor

bnjjj commented Nov 9, 2022

By default we decided to disable the export of trace_id in response headers for several reasons.
The goal of this issue is to find what we need to configure the behavior of trace_id.

Here are 3 different features I detected:

  • Expose or not the trace_id in response headers
  • Specify the header name if we want to expose it and modify the default one (default: apollo-trace-id)
  • Specify a custom propagator to indicate what's the header name to set the trace_id

Configuration example:

telemetry:
  tracing:
    experimental_expose_trace_id:
      enabled: true # default: false
      header_name: "my-trace-id" # default: "apollo-trace-id"
    propagation:
      custom_header: "x-request-id"
      jaeger: true
@bnjjj bnjjj self-assigned this Nov 9, 2022
@BrynCooke
Copy link
Contributor

Suggest that experimental_expose_trace_id is merged into the logging section in #2040

Maybe rename it to display_trace_id

header_name is maybe not be needed once propagation.custom_header is implemented as the trace id is available from the span.

@bnjjj
Copy link
Contributor Author

bnjjj commented Nov 21, 2022

I don't think the experimental_expose_trace_id should be in logging section, because it doesn't change the logging behavior, it will always display the trace_id in logs. It's all about the trace_id in response and request headers and then is still related only to trace. But if you think it might also be useful to have a way to not display the trace_id in logs, then I can also add a display_trace_id in logging section because this is related to logging.

And about header_name, as it's not inside propagation.custom_header but directly inside experimental_expose_trace_id it's not so obvious that the name is supposed to be a header name.

bnjjj added a commit that referenced this issue Nov 24, 2022
close #2080 

Configuration example:

```yaml
telemetry:
  tracing:
    experimental_response_trace_id:
      enabled: true # default: false
      header_name: "my-trace-id" # default: "apollo-trace-id"
    propagation:
      request: 
        header_name: "x-request-id"
      jaeger: true
```

Signed-off-by: Benjamin Coenen <[email protected]>
Co-authored-by: Geoffroy Couprie <[email protected]>
garypen pushed a commit that referenced this issue Nov 30, 2022
close #2080 

Configuration example:

```yaml
telemetry:
  tracing:
    experimental_response_trace_id:
      enabled: true # default: false
      header_name: "my-trace-id" # default: "apollo-trace-id"
    propagation:
      request: 
        header_name: "x-request-id"
      jaeger: true
```

Signed-off-by: Benjamin Coenen <[email protected]>
Co-authored-by: Geoffroy Couprie <[email protected]>
@BrynCooke BrynCooke modified the milestones: v1-NEXT, v1.5.0 Dec 2, 2022
@garypen garypen added this to the v1.5.0 milestone Dec 5, 2022
@BrynCooke BrynCooke modified the milestone: v1.5.0 Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants