fix: allow rename of structuremetadata labels #13955
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -327,20 +327,52 @@ func (b *LabelsBuilder) Get(key string) (string, bool) { | |
| // Del deletes the label of the given name. | ||
| func (b *LabelsBuilder) Del(ns ...string) *LabelsBuilder { | ||
| for _, n := range ns { | ||
| for category := range b.add { | ||
| b.deleteWithCategory(LabelCategory(category), n) | ||
| } | ||
| b.del = append(b.del, n) | ||
| } | ||
| return b | ||
| } | ||
|
|
||
| // deleteWithCategory removes the label from the specified category | ||
| func (b *LabelsBuilder) deleteWithCategory(category LabelCategory, n string) { | ||
| for i, l := range b.add[category] { | ||
| if l.Name == n { | ||
| b.add[category] = append(b.add[category][:i], b.add[category][i+1:]...) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Set the name/value pair as a label. | ||
| // The value `v` may not be set if a category with higher preference already contains `n`. | ||
| // Category preference goes as Parsed > Structured Metadata > Stream. | ||
| func (b *LabelsBuilder) Set(category LabelCategory, n, v string) *LabelsBuilder { | ||
| // Parsed takes precedence over Structured Metadata and Stream labels. | ||
| // If category is Parsed, we delete `n` from the structured metadata and stream labels. | ||
| if category == ParsedLabel { | ||
| b.deleteWithCategory(StructuredMetadataLabel, n) | ||
| b.deleteWithCategory(StreamLabel, n) | ||
| } | ||
|
|
||
| // Structured Metadata takes precedence over Stream labels. | ||
| // If category is `StructuredMetadataLabel`,we delete `n` from the stream labels. | ||
| // If `n` exists in the parsed labels, we won't overwrite it's value and we just return what we have. | ||
| if category == StructuredMetadataLabel { | ||
| b.deleteWithCategory(StreamLabel, n) | ||
| if labelsContain(b.add[ParsedLabel], n) { | ||
| return b | ||
| } | ||
| } | ||
|
|
||
| // Finally, if category is `StreamLabel` and `n` already exists in either the structured metadata or | ||
| // parsed labels, the `Set` operation is a noop and we return the unmodified labels builder. | ||
| if category == StreamLabel { | ||
| if labelsContain(b.add[StructuredMetadataLabel], n) || labelsContain(b.add[ParsedLabel], n) { | ||
| return b | ||
| } | ||
| } | ||
|
|
||
| for i, a := range b.add[category] { | ||
| if a.Name == n { | ||
| b.add[category][i].Value = v | ||
|
|
@@ -430,6 +462,7 @@ func (b *LabelsBuilder) UnsortedLabels(buf labels.Labels, categories ...LabelCat | |
| } else { | ||
| buf = buf[:0] | ||
| } | ||
|
|
||
| if categoriesContain(categories, StreamLabel) { | ||
| Outer: | ||
| for _, l := range b.base { | ||
|
|
@@ -439,20 +472,38 @@ func (b *LabelsBuilder) UnsortedLabels(buf labels.Labels, categories ...LabelCat | |
| continue Outer | ||
| } | ||
| } | ||
|
|
||
| // Skip stream labels which value will be replaced by structured metadata | ||
| if labelsContain(b.add[StructuredMetadataLabel], l.Name) { | ||
| continue | ||
| } | ||
|
|
||
| // Skip stream labels which value will be replaced by parsed labels | ||
| if labelsContain(b.add[ParsedLabel], l.Name) { | ||
| continue | ||
| } | ||
|
|
||
| // Take value from stream label if present | ||
| if labelsContain(b.add[StreamLabel], l.Name) { | ||
| buf = append(buf, labels.Label{Name: l.Name, Value: b.add[StreamLabel].Get(l.Name)}) | ||
|
There was a problem hiding this comment. As you pointed out, the StreamLabel labels are not effectively used so the code will never get here, but good that you added the check. There was a problem hiding this comment. @salvacorts Once this PR is merged I am planning to raise a separate PR, Removing the stream label and separating the |
||
| } else { | ||
| buf = append(buf, l) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if categoriesContain(categories, StructuredMetadataLabel) { | ||
| for _, l := range b.add[StructuredMetadataLabel] { | ||
| if labelsContain(b.add[ParsedLabel], l.Name) { | ||
| continue | ||
| } | ||
|
|
||
| buf = append(buf, l) | ||
| } | ||
| } | ||
|
|
||
| if categoriesContain(categories, ParsedLabel) { | ||
| buf = append(buf, b.add[ParsedLabel]...) | ||
| } | ||
| if (b.HasErr() || b.HasErrorDetails()) && categoriesContain(categories, ParsedLabel) { | ||
| buf = b.appendErrors(buf) | ||
|
|
@@ -566,6 +617,15 @@ func flattenLabels(buf labels.Labels, many ...labels.Labels) labels.Labels { | |
| return buf | ||
| } | ||
|
|
||
| func labelsContain(labels labels.Labels, name string) bool { | ||
| for _, l := range labels { | ||
| if l.Name == name { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| func (b *BaseLabelsBuilder) toUncategorizedResult(buf labels.Labels) LabelsResult { | ||
| hash := b.hasher.Hash(buf) | ||
| if cached, ok := b.resultCache[hash]; ok { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 think the Set operation is kinda misleading as it may not set the actual value.
RankedSetwould be abetter name but renaming all the occurrences will be a pain. For now I think we are good as long as we leave a comment explaining how it works.I'd add this comment to the top of the method\
Then for each category check I'd add these comments:
For
category == ParsedLabelFor
category == StructuredMetadataLabelFor
category == StreamLabel