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 support for x scope org id header in loki source api #1805

Conversation

QuentinBisson
Copy link
Contributor

@QuentinBisson QuentinBisson commented Oct 2, 2024

PR Description

This pull request introduces a middleware for extracting tenant headers in the loki PushAPIServer to improve tenant context handling.

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@QuentinBisson
Copy link
Contributor Author

I also added tests :)

@mattdurham
Copy link
Collaborator

Can we add documentation to the loki.source.api component?

@QuentinBisson
Copy link
Contributor Author

@mattdurham do you want me to document the use of the header? Sure I will do that tomorrow at the latest

@mattdurham
Copy link
Collaborator

Yes, maybe something like we did with scrape https://grafana.com/docs/alloy/latest/reference/components/prometheus/prometheus.scrape/#technical-details

@clayton-cornell would be the expert on how this should be organized.

@clayton-cornell
Copy link
Contributor

Lets keep it simple. If it's a technical description of how things work and what to expect... then following the prometheus.scrape example should work. I'll review and if there's structural changes we can either do it in this PR or merge this one and I'll rework in a new (whichever is easiest).

@QuentinBisson
Copy link
Contributor Author

@clayton-cornell Could you check if the documentation is alright?

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
Copy link
Contributor

@clayton-cornell clayton-cornell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the docs are OK for this PR.

I will open up an issue on the layout and structure of the Technical details section. I think it's at the wrong level in the topic hierarchy... here and in prometheus.scrape. Instead of blocking this PR for the doc fixes, let's merge and I'll fix things in a separate doc-only PR.

@QuentinBisson
Copy link
Contributor Author

Thanks for the help :)

Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mattdurham mattdurham enabled auto-merge (squash) October 2, 2024 19:49
@mattdurham mattdurham merged commit 35e256f into grafana:main Oct 2, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants