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.

Sign up for GitHub

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

Jump to bottom

[receiver/splunkhec] Add support passing token via context in splunkhecreceiver #37406

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[receiver/splunkhec] Add support passing token via context in splunkhecreceiver #37406

wants to merge 3 commits into from

Conversation

CodePrometheus
Copy link
Contributor

@CodePrometheus CodePrometheus commented Jan 22, 2025 •
edited
Loading

Description

Link to tracking issue

Fixes #37307

Testing

Documentation

@CodePrometheus CodePrometheus requested review from atoulme, a team and dmitryax as code owners January 22, 2025 06:36
asreehari-splunk
asreehari-splunk reviewed Jan 23, 2025
View reviewed changes
receiver/splunkhecreceiver/receiver.go Outdated
if strings.HasPrefix(accessToken, splunk.HECTokenHeader+" ") {
accessTokenValue := accessToken[len(splunk.HECTokenHeader)+1:]
if tokenContextGate.IsEnabled() {
return context.WithValue(origCtx, splunk.LabelType(splunk.HecTokenLabel), accessTokenValue), nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Its unclear why a new label is being introduced here. We want to move towards using X-Sf-token as the label so that when this receiver is used with say the batch processor, it will by default group data associated with the token. The default configuration for the batch processor is in the agent.yaml. Can you please clarify ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it's my oversight, let me fix it.

@asreehari-splunk
Copy link
Contributor

Thank you for the making the change. For completeness, we want to do one more step which is an end to end test. pair this receiver with a processor and exporter pair to confirm that the functionality is actually working as intended. Also, please share how you verified the working. thats critical to move forward on this.

also while you have a link to the issue this MR is intending to fix, please add a clear description on what you are trying to do

@CodePrometheus
Copy link
Contributor Author

@asreehari-splunk The general steps for e2e:

  • Use telemetrygen to send metrics and configure the exporter upstream so that the header contains the value of the Splunk prefix
  • otlp receiver with include_metadata set to truereceiver.splunkhec.passtokenviacontext enabled and access_token_passthrough set to true
  • batch processor using the X-Sf-Token key to group

also while you have a link to the issue this MR is intending to fix, please add a clear description on what you are trying to do

Is .chloggen/passing-token-via-context-splunkhec-receiver.yaml enough?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asreehari-splunk asreehari-splunk asreehari-splunk left review comments

@atoulme atoulme Awaiting requested review from atoulme atoulme is a code owner

@dmitryax dmitryax Awaiting requested review from dmitryax dmitryax is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@MovieStoreGuy MovieStoreGuy

Labels
internal/splunk receiver/splunkhec
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[receiver/splunkhec] Support passing access token via context
3 participants
@CodePrometheus @asreehari-splunk @MovieStoreGuy