Skip to content

Conversation

@pracucci
Copy link
Collaborator

@pracucci pracucci commented Mar 4, 2024

What this PR does

While working on #7529, I've noticed that we have utilities to add "source IP" in the gRPC metadata but we never read it (I've also checked GEM). This was introduced almost 4 years ago in Cortex (see PR): at that time we were also reading it in Cortex. I've the feeling this code is actually dead code no more used, so we can remove it (proposed in this PR), but please block me if you're aware such metadata is actually somewhere.

Which issue(s) this PR fixes or relates to

N/A

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

Copy link
Contributor

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

LGTM, I also couldn't find any references to GetSourceIPsFromOutgoingCtx in GH code search within grafana repos https://github.com/search?type=code&q=GetSourceIPsFromOutgoingCtx

@pracucci pracucci force-pushed the remove-x-forwarded-for-context-propagation branch from fcaeee2 to 9db9c90 Compare May 6, 2024 08:44
@pracucci pracucci marked this pull request as ready for review May 6, 2024 08:44
@pracucci pracucci requested a review from a team as a code owner May 6, 2024 08:44
@pracucci
Copy link
Collaborator Author

pracucci commented May 6, 2024

I double checked it both in Mimir and GEM and I can't find any usage of these functions. I proceed merging this PR.

@pracucci pracucci enabled auto-merge (squash) May 6, 2024 08:44
@pracucci pracucci merged commit 7d6b067 into main May 6, 2024
@pracucci pracucci deleted the remove-x-forwarded-for-context-propagation branch May 6, 2024 08:58
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