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

WIP: Datadog Baggage API #3069

Merged
merged 10 commits into from
Jan 16, 2025
Merged

WIP: Datadog Baggage API #3069

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
be5ed6c
WIP: Baggage API
rachelyangdog Jan 8, 2025
39a9f0d
copyright headers
rachelyangdog Jan 8, 2025
3727d04
resolving comments
rachelyangdog Jan 10, 2025
a481ced
adding test case for ok
rachelyangdog Jan 10, 2025
4b088e7
Update ddtrace/tracer/baggage_test.go
rachelyangdog Jan 13, 2025
ebc4810
adding ok idiom to baggageMap
rachelyangdog Jan 13, 2025
b2f8b7b
renaming and adding folder
rachelyangdog Jan 14, 2025
e6a6940
package name
rachelyangdog Jan 15, 2025
2cfb7b3
change name for getAll
rachelyangdog Jan 15, 2025
1502f94
Merge branch 'main' into rachel.yang/baggage-api-context
rachelyangdog Jan 16, 2025
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
20 changes: 10 additions & 10 deletions ddtrace/tracer/baggage.go → ddtrace/baggage/baggage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ func withBaggage(ctx context.Context, baggage map[string]string) context.Context
return context.WithValue(ctx, baggageKey{}, baggage)
}

// SetBaggage sets or updates a single baggage key/value pair in the context.
// Set sets or updates a single baggage key/value pair in the context.
// If the key already exists, this function overwrites the existing value.
func SetBaggage(ctx context.Context, key, value string) context.Context {
func Set(ctx context.Context, key, value string) context.Context {
bm, ok := baggageMap(ctx)
if !ok {
// If there's no baggage map yet, create one
Expand All @@ -42,9 +42,9 @@ func SetBaggage(ctx context.Context, key, value string) context.Context {
return withBaggage(ctx, bm)
}

// Baggage retrieves the value associated with a baggage key.
// Get retrieves the value associated with a baggage key.
// If the key isn't found, it returns an empty string.
func Baggage(ctx context.Context, key string) (string, bool) {
func Get(ctx context.Context, key string) (string, bool) {
bm, ok := baggageMap(ctx)
if !ok {
return "", false
Expand All @@ -53,8 +53,8 @@ func Baggage(ctx context.Context, key string) (string, bool) {
return value, ok
}

// RemoveBaggage removes the specified key from the baggage (if present).
func RemoveBaggage(ctx context.Context, key string) context.Context {
// Remove removes the specified key from the baggage (if present).
func Remove(ctx context.Context, key string) context.Context {
bm, ok := baggageMap(ctx)
if !ok {
// nothing to remove
Expand All @@ -64,8 +64,8 @@ func RemoveBaggage(ctx context.Context, key string) context.Context {
return withBaggage(ctx, bm)
}

// AllBaggage returns a **copy** of all baggage items in the context,
func AllBaggage(ctx context.Context) map[string]string {
// GetAll returns a **copy** of all baggage items in the context,
func GetAll(ctx context.Context) map[string]string {
Copy link
Member

Choose a reason for hiding this comment

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

Feel free to ignore the following: although GetAll is clear enough, I feel a more idiomatic naming would be All - like in slices.All - or Items/Entries - for which I don't have any example in the stdlib but it could be aligned with Values present in multiple stdlib packages.

Anyway, I'm approving this as is.

@mtoffl01 WDYT about the current API?

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be confusing because the function lives on a package called tracer; if we moved baggage into its own package, then we could do baggage.All.
Actually GetAll is still ambiguous on the tracer package; what about renaming it to AllBaggage? Either that, or move all this code into a package called baggage, and then rename the function to All

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code has been moved to a separate package called baggage! Now lives under ddtrace and not tracer.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still seeing package tracer at the top; should be package baggage, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah! just saw it and updated it

bm, ok := baggageMap(ctx)
if !ok {
return nil
Expand All @@ -77,7 +77,7 @@ func AllBaggage(ctx context.Context) map[string]string {
return copyMap
}

// ClearBaggage completely removes all baggage items from the context.
func ClearBaggage(ctx context.Context) context.Context {
// Clear completely removes all baggage items from the context.
func Clear(ctx context.Context) context.Context {
return withBaggage(ctx, nil)
}
44 changes: 22 additions & 22 deletions ddtrace/tracer/baggage_test.go → ddtrace/baggage/baggage_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ import (
)

func TestBaggageFunctions(t *testing.T) {
t.Run("SetBaggage and Baggage", func(t *testing.T) {
t.Run("Set and Get", func(t *testing.T) {
ctx := context.Background()

// Set a key/value in the baggage
ctx = SetBaggage(ctx, "foo", "bar")
ctx = Set(ctx, "foo", "bar")

// Retrieve that value
got, ok := Baggage(ctx, "foo")
got, ok := Get(ctx, "foo")
if !ok {
t.Error("Expected key \"foo\" to be found in baggage, got ok=false")
}
Expand All @@ -27,7 +27,7 @@ func TestBaggageFunctions(t *testing.T) {
}

// Ensure retrieving a non-existent key returns an empty string and false
got, ok = Baggage(ctx, "missingKey")
got, ok = Get(ctx, "missingKey")
if ok {
t.Error("Expected key \"missingKey\" to not be found, got ok=true")
}
Expand All @@ -36,15 +36,15 @@ func TestBaggageFunctions(t *testing.T) {
}
})

t.Run("AllBaggage", func(t *testing.T) {
t.Run("GetAll", func(t *testing.T) {
ctx := context.Background()

// Set multiple baggage entries
ctx = SetBaggage(ctx, "key1", "value1")
ctx = SetBaggage(ctx, "key2", "value2")
ctx = Set(ctx, "key1", "value1")
ctx = Set(ctx, "key2", "value2")

// Retrieve all baggage entries
all := AllBaggage(ctx)
all := GetAll(ctx)
if len(all) != 2 {
t.Fatalf("Expected 2 items in baggage; got %d", len(all))
}
Expand All @@ -59,23 +59,23 @@ func TestBaggageFunctions(t *testing.T) {

// Confirm returned map is a copy, not the original
all["key1"] = "modified"
val, _ := Baggage(ctx, "key1")
val, _ := Get(ctx, "key1")
if val == "modified" {
t.Error("AllBaggage returned a map that mutates the original baggage!")
}
})

t.Run("RemoveBaggage", func(t *testing.T) {
t.Run("Remove", func(t *testing.T) {
ctx := context.Background()

// Add baggage to remove
ctx = SetBaggage(ctx, "deleteMe", "toBeRemoved")
ctx = Set(ctx, "deleteMe", "toBeRemoved")

// Remove it
ctx = RemoveBaggage(ctx, "deleteMe")
ctx = Remove(ctx, "deleteMe")

// Verify removal
got, ok := Baggage(ctx, "deleteMe")
got, ok := Get(ctx, "deleteMe")
if ok {
t.Error("Expected key \"deleteMe\" to be removed, got ok=true")
}
Expand All @@ -84,18 +84,18 @@ func TestBaggageFunctions(t *testing.T) {
}
})

t.Run("ClearBaggage", func(t *testing.T) {
t.Run("Clear", func(t *testing.T) {
ctx := context.Background()

// Add multiple items
ctx = SetBaggage(ctx, "k1", "v1")
ctx = SetBaggage(ctx, "k2", "v2")
ctx = Set(ctx, "k1", "v1")
ctx = Set(ctx, "k2", "v2")

// Clear all baggage
ctx = ClearBaggage(ctx)
ctx = Clear(ctx)

// Check that everything is gone
all := AllBaggage(ctx)
all := GetAll(ctx)
if len(all) != 0 {
t.Errorf("Expected no items after clearing baggage; got %d", len(all))
}
Expand All @@ -109,7 +109,7 @@ func TestBaggageFunctions(t *testing.T) {
ctx = withBaggage(ctx, initialMap)

// Verify
got, _ := Baggage(ctx, "customKey")
got, _ := Get(ctx, "customKey")
if got != "customValue" {
t.Errorf("Baggage(ctx, \"customKey\") = %q; want \"customValue\"", got)
}
Expand All @@ -119,13 +119,13 @@ func TestBaggageFunctions(t *testing.T) {
ctx := context.Background()

// Check an unset key
val, ok := Baggage(ctx, "unsetKey")
val, ok := Get(ctx, "unsetKey")
if ok {
t.Errorf("Expected unset key to return ok=false, got ok=true with val=%q", val)
}

ctx = SetBaggage(ctx, "testKey", "testVal")
val, ok = Baggage(ctx, "testKey")
ctx = Set(ctx, "testKey", "testVal")
val, ok = Get(ctx, "testKey")
if !ok {
t.Error("Expected key \"testKey\" to be present, got ok=false")
}
Expand Down
Loading