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

Introduce a flag that skips tag duplicate removal logic #165

Merged
merged 1 commit into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ type Engine struct {
// that manipulates this field directly has to respect this requirement.
Tags []Tag

// Indicates whether to allow duplicated tags from the tags list before sending.
// This option is turned off by default, ensuring that duplicate tags are removed.
// Turn it on if you need to send the same tag multiple times with different values,
// which is a special use case.
AllowDuplicateTags bool

// This cache keeps track of the generated measure structures to avoid
// rebuilding them every time a same measure type is seen by the engine.
//
Expand Down Expand Up @@ -148,7 +154,7 @@ func (eng *Engine) measure(t time.Time, name string, value interface{}, ftype Fi
m.Tags = append(m.Tags[:0], eng.Tags...)
m.Tags = append(m.Tags, tags...)

if len(tags) != 0 && !TagsAreSorted(m.Tags) {
if len(tags) != 0 && !eng.AllowDuplicateTags && !TagsAreSorted(m.Tags) {
SortTags(m.Tags)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still want to sort tags even if we are not removing duplicates?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we rename the SortTags function to RemoveDuplicates that's really what it's doing, the sorting is an artifact of that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It makes sense but I can see other usages such as flagon. Do you want to rename to RemoveDuplicates because it doesn't offer the backwards compatibility?

}

Expand Down Expand Up @@ -192,7 +198,9 @@ func (eng *Engine) ReportAt(time time.Time, metrics interface{}, tags ...Tag) {
tb = tagsPool.Get().(*tagsBuffer)
tb.append(tags...)
tb.append(eng.Tags...)
tb.sort()
if !eng.AllowDuplicateTags {
tb.sort()
}
tags = tb.tags
}

Expand Down
27 changes: 27 additions & 0 deletions engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ func TestEngine(t *testing.T) {
scenario: "calling Engine.WithTags produces expected tags",
function: testEngineWithTags,
},
{
scenario: "calling Engine.Incr produces expected tags when AllowDuplicateTags is set",
function: testEngineAllowDuplicateTags,
},
}

for _, test := range tests {
Expand Down Expand Up @@ -126,6 +130,29 @@ func testEngineFlush(t *testing.T, eng *stats.Engine) {
}
}

func testEngineAllowDuplicateTags(t *testing.T, eng *stats.Engine) {
e2 := eng.WithTags()
e2.AllowDuplicateTags = true
if e2.Prefix != "test" {
t.Error("bad prefix:", e2.Prefix)
}
e2.Incr("measure.count")
e2.Incr("measure.count", stats.T("category", "a"), stats.T("category", "b"), stats.T("category", "c"))

checkMeasuresEqual(t, e2,
stats.Measure{
Name: "test.measure",
Fields: []stats.Field{stats.MakeField("count", 1, stats.Counter)},
Tags: []stats.Tag{stats.T("service", "test-service")},
},
stats.Measure{
Name: "test.measure",
Fields: []stats.Field{stats.MakeField("count", 1, stats.Counter)},
Tags: []stats.Tag{stats.T("service", "test-service"), stats.T("category", "a"), stats.T("category", "b"), stats.T("category", "c")},
},
)
}

func testEngineIncr(t *testing.T, eng *stats.Engine) {
eng.Incr("measure.count")
eng.Incr("measure.count", stats.T("type", "testing"))
Expand Down
7 changes: 3 additions & 4 deletions tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,9 @@ func TagsAreSorted(tags []Tag) bool {
return slices.IsSortedFunc(tags, tagCompare)
}

// SortTags sorts and deduplicates tags in-place,
// favoring later elements whenever a tag name duplicate occurs.
// The returned slice may be shorter than the input
// due to the elimination of duplicates.
// SortTags sorts and deduplicates tags in-place, favoring later elements
// whenever a tag name duplicate occurs. The returned slice may be shorter than
// the input due to the elimination of duplicates.
func SortTags(tags []Tag) []Tag {
// Stable sort ensures that we have deterministic
// "latest wins" deduplication.
Expand Down
Loading