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

Custom http headers for tracing #7266

Closed
krmcbride opened this issue Apr 8, 2024 · 5 comments · Fixed by #7353
Closed

Custom http headers for tracing #7266

krmcbride opened this issue Apr 8, 2024 · 5 comments · Fixed by #7353
Assignees
Labels
backlog Triaged Issue added to backlog enhancement This is the preferred way to describe new end-to-end features. requires operator PR It requires update in operator code

Comments

@krmcbride
Copy link

What do you want to improve?

Similar to #4323 but for the tracing config -- I'm using a multitenant Grafana Tempo deployment which needs a X-Scope-OrgID header set with a tenant ID. I can connect to Grafana Mimir with the prometheus.custom_headers config:

spec:
  # ...
  external_services:
    prometheus:
      custom_headers:
        "X-Scope-OrgID": "<the tenant id>"

But there doesn't appear to be an equivalent for tracing/tempo.

What is the current behavior?

Requests to Tempo without the X-Scope-OrgID set result in a

HTTP/1.1 401 Unauthorized
Content-Type: text/plain; charset=utf-8

no org id

response, which causes a JSON unmarshalling error.

As a temp workaround I'm proxying requests from Kiali to Tempo through an nginx pod with

location ^~ /api {
  proxy_pass        https://my-grafana-tempo-url$request_uri;
  proxy_set_header  X-Scope-OrgID "<the tenant id>";
}

to set the correct tenant header, and trace info appears in Kiali as expected.

What is the new behavior?

Logically I'd expect there to be an optional spec.external_services.tracing.custom_headers map, same as for the prometheus client.

@krmcbride krmcbride added the enhancement This is the preferred way to describe new end-to-end features. label Apr 8, 2024
@jshaughn jshaughn added the refinement Still being full defined before taking any action label Apr 15, 2024
@jshaughn
Copy link
Collaborator

I think this makes sense. We support query_scope, but that adds tags to queries in order to narrow the queried data, when trace data is unified. This is different as it is adding headers.

I will add this to the backlog, @krmcbride, would you be interested in contributing this feature?

@jshaughn jshaughn added backlog Triaged Issue added to backlog and removed refinement Still being full defined before taking any action labels Apr 15, 2024
@jshaughn jshaughn added the help wanted Needs a contributor. Useful but outside maintainer backlog label May 5, 2024
@jmazzitelli jmazzitelli removed the help wanted Needs a contributor. Useful but outside maintainer backlog label May 13, 2024
@jmazzitelli jmazzitelli self-assigned this May 13, 2024
@jmazzitelli
Copy link
Collaborator

I did the original impl for the prom headers, so I can try to do this since its very similar

jmazzitelli added a commit to jmazzitelli/kiali-operator that referenced this issue May 13, 2024
jmazzitelli added a commit to jmazzitelli/kiali that referenced this issue May 13, 2024
jmazzitelli added a commit to jmazzitelli/kiali that referenced this issue May 13, 2024
jmazzitelli added a commit to jmazzitelli/kiali that referenced this issue May 15, 2024
@jmazzitelli
Copy link
Collaborator

@krmcbride Do you want to test this locally to see if it addresses your issue? I uploaded the dev images on my quay repo:

The new configuration should look like:

external_services:
  tracing:
    custom_headers:
      HeaderName1: HeaderValue1

@jmazzitelli jmazzitelli added the requires operator PR It requires update in operator code label May 15, 2024
jmazzitelli added a commit to jmazzitelli/kiali that referenced this issue May 17, 2024
@jmazzitelli
Copy link
Collaborator

The two PRs are ready for review. In my testing, I can see the custom headers get passed using kubeshark to examine the requests.

@krmcbride
Copy link
Author

Thanks @jmazzitelli, tested your issue7266 images in a sandbox cluster with the expected tempo custom_headers and it works great 👍

jmazzitelli added a commit that referenced this issue May 22, 2024
* send custom headers in tracing backend requests
fixes: #7266

* add ctx to the tempo grpc client
jmazzitelli added a commit to kiali/kiali-operator that referenced this issue May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Triaged Issue added to backlog enhancement This is the preferred way to describe new end-to-end features. requires operator PR It requires update in operator code
Projects
Development

Successfully merging a pull request may close this issue.

3 participants