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

Expose Kafka message key in labels #4745

Merged
merged 7 commits into from
Nov 19, 2021

Conversation

taisho6339
Copy link
Contributor

@taisho6339 taisho6339 commented Nov 12, 2021

What this PR does / why we need it:

Expose Kafka message key in label set and add description into the documents.

Which issue(s) this PR fixes:
Fixes #4728

Special notes for your reviewer:

Checklist

  • Documentation added
  • Tests updated
  • Add an entry in the CHANGELOG.md about the changes.

@taisho6339 taisho6339 marked this pull request as ready for review November 12, 2021 16:23
@taisho6339 taisho6339 marked this pull request as draft November 13, 2021 03:39
@pull-request-size pull-request-size bot added size/L and removed size/S labels Nov 13, 2021
@CLAassistant
Copy link

CLAassistant commented Nov 13, 2021

CLA assistant check
All committers have signed the CLA.

@taisho6339 taisho6339 force-pushed the feature/issue/4728 branch 2 times, most recently from 5efab84 to da986b7 Compare November 13, 2021 05:17
@taisho6339 taisho6339 marked this pull request as ready for review November 13, 2021 05:48
labelKeyKafkaMessageKey = "__meta_kafka_message_key"
)

func (t *Target) messageKeyLabel() (useLabel bool, labelName model.LabelName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should pass the actual value to the format and not a default since relabeling can match regexes to apply change or not.

In fact all discovered labels should be there since you can compute multiple label values using source_labels but that sounds even trickier.

Let's start with passing the value and we'll see how we can handle the rest later. (Possibly relabel everything twice, once for the target labels, once for the message)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, you're right.

Let's start with passing the value

Anyway, let me do that

@taisho6339
Copy link
Contributor Author

@cyriltovena

I've fixed to pass actual value in message key and left a TODO comment for #4745 (comment).

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@cyriltovena cyriltovena merged commit d61dd18 into grafana:main Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kafka message key should be exposed.
3 participants