-
Notifications
You must be signed in to change notification settings - Fork 530
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
Fix multitenancy when using OTLP HTTP #1781
Fix multitenancy when using OTLP HTTP #1781
Conversation
2a7b8cc
to
7839cd6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks okay to me, thanks for the PR. I'll give my teammates a chance to review also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Could you add a changelog entry?
7839cd6
to
391aa3b
Compare
Done! |
info := client.FromContext(ctx) | ||
orgIDs := info.Metadata.Get(user.OrgIDHeaderName) | ||
if len(orgIDs) != 1 { | ||
log.Logger.Log("msg", "failed to extract org id", "err", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add more detail to this error message? now it represents 2 errors in a row. failure to pull the orgid from grpc and failure to pull the orgid from http. perhaps we can log the value of orgIDs
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the error message @joe-elliott
But note that https://github.com/weaveworks/common/blob/master/user/grpc.go#L8-L22 the GRPC extractor doesn't log when multiple orgids are present.
2ca81ec
to
4c9e645
Compare
Fixes grafana#495 Signed-off-by: Goutham Veeramachaneni <[email protected]>
Signed-off-by: Goutham Veeramachaneni <[email protected]>
4c9e645
to
50f8b0e
Compare
Thanks @gouthamve! I think you just fixed one of our oldest open issues 🙏 |
* Fix multitenancy when using OTLP HTTP Fixes grafana#495 Signed-off-by: Goutham Veeramachaneni <[email protected]> * review feedback Signed-off-by: Goutham Veeramachaneni <[email protected]> Signed-off-by: Goutham Veeramachaneni <[email protected]>
great👍,thanks |
Which issue(s) this PR fixes:
Fixes #495
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]