Skip to content

Commit

Permalink
Fix bug with parsing labels from baggage
Browse files Browse the repository at this point in the history
Previously we would still try set profiler labels even if incoming
baggage had no k6 metadata. This skips setting profiler labels with no
k6 metadata.
  • Loading branch information
bryanhuhta committed Dec 20, 2024
1 parent 293e09f commit 8477ab7
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 4 deletions.
12 changes: 8 additions & 4 deletions x/k6/baggage.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,11 @@ func getBaggageLabelsFromContext(ctx context.Context) *pyroscope.LabelSet {
return nil
}

labels := baggageToLabels(b)
return &labels
return baggageToLabels(b)
}

// baggageToLabels converts request baggage to a LabelSet.
func baggageToLabels(b baggage.Baggage) pyroscope.LabelSet {
func baggageToLabels(b baggage.Baggage) *pyroscope.LabelSet {
labelPairs := make([]string, 0, len(b.Members())*2)
for _, m := range b.Members() {
if !strings.HasPrefix(m.Key(), "k6.") {
Expand All @@ -134,5 +133,10 @@ func baggageToLabels(b baggage.Baggage) pyroscope.LabelSet {
labelPairs = append(labelPairs, key, m.Value())
}

return pyroscope.Labels(labelPairs...)
if len(labelPairs) == 0 {
return nil
}

labels := pyroscope.Labels(labelPairs...)
return &labels
}
53 changes: 53 additions & 0 deletions x/k6/baggage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,41 @@ func Test_getBaggageLabels(t *testing.T) {
})
}

func Test_baggageToLabels(t *testing.T) {
t.Run("with_k6_baggage", func(t *testing.T) {
b := testMustNewBaggage(t,
"k6.test_run_id", "123",
"not_k6.some_other_key", "value",
)

labelSet := baggageToLabels(b)
require.NotNil(t, labelSet)

gotLabels := testPprofLabelsToMap(t, *labelSet)
expectedLabels := map[string]string{
"k6_test_run_id": "123",
}

require.Equal(t, expectedLabels, gotLabels)
})

t.Run("with_empty_baggage", func(t *testing.T) {
b := testMustNewBaggage(t)

labelSet := baggageToLabels(b)
require.Nil(t, labelSet)
})

t.Run("with_no_k6_baggage", func(t *testing.T) {
b := testMustNewBaggage(t,
"not_k6.some_other_key", "value",
)

labelSet := baggageToLabels(b)
require.Nil(t, labelSet)
})
}

func testAddBaggageToRequest(t *testing.T, req *http.Request, kvPairs ...string) *http.Request {
t.Helper()

Expand All @@ -157,6 +192,24 @@ func testAddBaggageToRequest(t *testing.T, req *http.Request, kvPairs ...string)
return req
}

func testMustNewBaggage(t *testing.T, kvPairs ...string) baggage.Baggage {
t.Helper()

require.Equal(t, 0, len(kvPairs)%2, "kvPairs must be a multiple of 2")

members := make([]baggage.Member, 0, len(kvPairs)/2)
for i := 0; i < len(kvPairs); i += 2 {
key := kvPairs[i]
value := kvPairs[i+1]
members = append(members, testMustNewMember(t, key, value))
}

b, err := baggage.New(members...)
require.NoError(t, err)

return b
}

func testMustNewMember(t *testing.T, key string, value string) baggage.Member {
t.Helper()

Expand Down

0 comments on commit 8477ab7

Please sign in to comment.