Support valkey-go tracing #3081

keisku · 2025-01-12T11:03:06Z

What does this PR do?

Support https://github.com/valkey-io/valkey-go

Example:

Motivation

#2920

Reviewer's Checklist

Changed code has unit tests for its functionality at or near 100% coverage.
System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
There is a benchmark for any new code, or changes to existing code.
If this interacts with the agent in a new way, a system test has been added.
Add an appropriate team label so this PR gets put in the right place for the release notes.
Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.
For internal contributors, a matching PR should be created to the v2-dev branch and reviewed by @DataDog/apm-go.

Unsure? Have a question? Request a review!

datadog-datadog-prod-us1 · 2025-01-12T11:09:50Z

Datadog Report

Branch report: keisku/valkey-go-support
Commit report: 3939a5a
Test service: dd-trace-go

❌ 8 Failed (0 Known Flaky), 5167 Passed, 70 Skipped, 2m 54.4s Total Time

❌ Failed Tests (8)

This report shows up to 5 failed tests.

TestNewClient - gopkg.in/DataDog/dd-trace-go.v1/contrib/valkey-go - Details
Expand for error
```
 Failed
 
 === RUN   TestNewClient
 --- FAIL: TestNewClient (0.11s)
```

TestNewClient/Test_GET_command_with_stream_with_env_vars - gopkg.in/DataDog/dd-trace-go.v1/contrib/valkey-go - Details

Expand for error

 Failed
 
 === RUN   TestNewClient/Test_GET_command_with_stream_with_env_vars
     valkey_test.go:287: 
         	Error Trace:	/home/runner/work/dd-trace-go/dd-trace-go/contrib/valkey-go/valkey_test.go:287
         	Error:      	Received unexpected error:
         	            	dial tcp 127.0.0.1:6380: connect: connection refused
         	Test:       	TestNewClient/Test_GET_command_with_stream_with_env_vars
         	Messages:   	Test GET command with stream with env vars
     --- FAIL: TestNewClient/Test_GET_command_with_stream_with_env_vars (0.02s)

TestNewClient/Test_HMGET_command_with_cache - gopkg.in/DataDog/dd-trace-go.v1/contrib/valkey-go - Details

Expand for error

 Failed
 
 === RUN   TestNewClient/Test_HMGET_command_with_cache
     valkey_test.go:287: 
         	Error Trace:	/home/runner/work/dd-trace-go/dd-trace-go/contrib/valkey-go/valkey_test.go:287
         	Error:      	Received unexpected error:
         	            	dial tcp 127.0.0.1:6380: connect: connection refused
         	Test:       	TestNewClient/Test_HMGET_command_with_cache
         	Messages:   	Test HMGET command with cache
     --- FAIL: TestNewClient/Test_HMGET_command_with_cache (0.01s)

TestNewClient/Test_invalid_password - gopkg.in/DataDog/dd-trace-go.v1/contrib/valkey-go - Details

Expand for error

 Failed
 
 === RUN   TestNewClient/Test_invalid_password
     valkey_test.go:67: 
         	Error Trace:	/home/runner/work/dd-trace-go/dd-trace-go/contrib/valkey-go/valkey_test.go:67
         	            				/home/runner/work/dd-trace-go/dd-trace-go/contrib/valkey-go/valkey_test.go:289
         	Error:      	Error message not equal:
         	            	expected: "WRONGPASS invalid username-password pair or user is disabled."
         	            	actual  : "dial tcp 127.0.0.1:6380: connect: connection refused"
         	Test:       	TestNewClient/Test_invalid_password
 ...

TestNewClient/Test_invalid_username - gopkg.in/DataDog/dd-trace-go.v1/contrib/valkey-go - Details

Expand for error

 Failed
 
 === RUN   TestNewClient/Test_invalid_username
     valkey_test.go:56: 
         	Error Trace:	/home/runner/work/dd-trace-go/dd-trace-go/contrib/valkey-go/valkey_test.go:56
         	            				/home/runner/work/dd-trace-go/dd-trace-go/contrib/valkey-go/valkey_test.go:289
         	Error:      	Error message not equal:
         	            	expected: "WRONGPASS invalid username-password pair or user is disabled."
         	            	actual  : "dial tcp 127.0.0.1:6380: connect: connection refused"
         	Test:       	TestNewClient/Test_invalid_username
 ...

pr-commenter · 2025-01-12T11:39:36Z

Benchmarks

Benchmark execution time: 2025-01-28 15:21:06

Comparing candidate commit 3234dc9 in PR branch keisku/valkey-go-support with baseline commit dde97eb in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 58 metrics, 1 unstable metrics.

contrib/valkey-go/valkey.go

darccio · 2025-01-17T14:58:09Z

@keisku Thanks for contributing this integration! Please fix the conflicts and this should be good to go.

Also remember to create the same PR against v2-dev. Keep in mind that this branch has some major differences, including an instrumentation package for contribs. Check the same integration for Redis to learn more about how to integrate a library in our contribs in our upcoming v2 version.

contrib/valkey-go/example_test.go

contrib/valkey-go/valkey.go

Signed-off-by: keisku <[email protected]>

contrib/valkey-go/valkey.go

rarguelloF · 2025-01-22T14:01:51Z

contrib/valkey-go/valkey.go

+	skipRawCommand bool
+}
+
+func (c *coreClient) peerTags() []tracer.StartSpanOption {


The agent checks a list of tags to compute peer.service, and I think both out.host and peer.hostname would be considered in this case. But for consistency with other integrations, I think its preferred to just set out.host (I am asking around if this preference has changed recently and if we have any documentation about it)

contrib/valkey-go/valkey.go

rarguelloF · 2025-01-22T16:22:55Z

contrib/valkey-go/valkey.go

+		opts = append(opts, []tracer.StartSpanOption{
+			// valkeyotel tags
+			tracer.Tag("db.stmt_size", input.size),
+			tracer.Tag("db.operation", input.command),


We don't need to add obfuscation, since its already done in the agent (the code you linked). However I just noticed there's an issue, since that code will only run if span.type is redis, so it won't run for valkey. I will talk to the apm-agent team about this.

But basically what we want to do here is:

if !input.skipRawCommand { opts = append(opts, tracer.Tag(ext.ValkeyRawCommand, input.command)) }

Also it is preferred to set only the raw command tag, since its the one the agent checks for obfuscation (when the logic is updated to check for valkey as well).

Signed-off-by: keisku <[email protected]>

rarguelloF

LGTM, thanks for the contribution! @keisku

datadog-datadog-prod-us1 · 2025-01-28T15:01:11Z

Datadog Report

Branch report: keisku/valkey-go-support
Commit report: d41d7ae
Test service: dd-trace-go

✅ 0 Failed, 5216 Passed, 72 Skipped, 2m 45.95s Total Time

github-actions bot added the apm:ecosystem contrib/* related feature requests or bugs label Jan 12, 2025

keisku force-pushed the keisku/valkey-go-support branch 8 times, most recently from e41ab88 to 7c97f5b Compare January 15, 2025 08:55

Support valkey-go tracing

6aa8d69

keisku force-pushed the keisku/valkey-go-support branch from 7c97f5b to 6aa8d69 Compare January 15, 2025 10:56

keisku marked this pull request as ready for review January 15, 2025 11:20

keisku requested review from a team as code owners January 15, 2025 11:20

keisku commented Jan 15, 2025

View reviewed changes

contrib/valkey-go/valkey.go Outdated Show resolved Hide resolved

keisku force-pushed the keisku/valkey-go-support branch from 11e7c51 to 2750600 Compare January 16, 2025 03:33

as per DD_APM_PEER_TAGS_AGGREGATION in agent

fe59e6d

keisku force-pushed the keisku/valkey-go-support branch from 2750600 to fe59e6d Compare January 16, 2025 03:36

Merge branch 'main' into keisku/valkey-go-support

26c6621

darccio approved these changes Jan 17, 2025

View reviewed changes

hannahkm requested changes Jan 17, 2025

View reviewed changes

contrib/valkey-go/example_test.go Show resolved Hide resolved

contrib/valkey-go/valkey.go Outdated Show resolved Hide resolved

keisku added 5 commits January 18, 2025 00:57

Merge remote-tracking branch 'origin' into keisku/valkey-go-support

9479b69

update example as per best practice

49ebeed

Signed-off-by: keisku <[email protected]>

fix doc

90f6c43

Signed-off-by: keisku <[email protected]>

fix spanName

46cacc3

Signed-off-by: keisku <[email protected]>

update contribIntegrations

6d5b6f6

Signed-off-by: keisku <[email protected]>

keisku added 7 commits January 21, 2025 00:42

for consistency with the other examples

aae0a85

remove ValkeyClientCommand* tags

14b4d21

there is not concept of db.instance in valkey / redis

6197ac1

remove db.type:valkey

7b7329f

remove db.application

fca4a18

fix valkey.raw_command behavior

e7758a3

add db.out tag

cdb4129

keisku requested a review from rarguelloF January 21, 2025 01:56

hannahkm approved these changes Jan 21, 2025

View reviewed changes

keisku mentioned this pull request Jan 21, 2025

Support obfuscation for valkey spans DataDog/datadog-agent#33187

Draft

rarguelloF requested changes Jan 22, 2025

View reviewed changes

keisku added 2 commits January 23, 2025 08:50

typo

21434c3

remove valkeyotel tags

27b0fec

keisku force-pushed the keisku/valkey-go-support branch from 787b31f to 27b0fec Compare January 23, 2025 08:51

keisku added 4 commits January 23, 2025 14:35

rename to out.db (ext.TargetDB)

06360d2

remove peer.* tags

b8c5c35

rework DD_TRACE_VALKEY_RAW_COMMAND behavior

fba84df

fix error handling when a span finishes

25088d1

keisku requested a review from rarguelloF January 23, 2025 15:08

rarguelloF and others added 4 commits January 27, 2025 17:29

fix raw command + add missing service name option + refactor

d9bb011

Merge branch 'main' into keisku/valkey-go-support

4ab8179

fix: go mod tidy smoke test

7eeeb6c

Signed-off-by: keisku <[email protected]>

Merge branch 'main' into keisku/valkey-go-support

b9e6353

rarguelloF approved these changes Jan 28, 2025

View reviewed changes

rarguelloF enabled auto-merge (squash) January 28, 2025 13:31

rarguelloF added 2 commits January 28, 2025 14:32

Merge branch 'main' into keisku/valkey-go-support

7c87d09

fix github workflows file

3234dc9

rarguelloF merged commit f0ed584 into main Jan 28, 2025
197 checks passed

rarguelloF deleted the keisku/valkey-go-support branch January 28, 2025 15:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support valkey-go tracing #3081

Support valkey-go tracing #3081

keisku commented Jan 12, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot commented Jan 12, 2025 •

edited

Loading

pr-commenter bot commented Jan 12, 2025 •

edited

Loading

darccio commented Jan 17, 2025

rarguelloF Jan 22, 2025 •

edited

Loading

rarguelloF Jan 22, 2025

rarguelloF left a comment

datadog-datadog-prod-us1 bot commented Jan 28, 2025 •

edited

Loading

Support valkey-go tracing #3081

Support valkey-go tracing #3081

Conversation

keisku commented Jan 12, 2025 • edited Loading

What does this PR do?

Motivation

Reviewer's Checklist

datadog-datadog-prod-us1 bot commented Jan 12, 2025 • edited Loading

Datadog Report

❌ Failed Tests (8)

pr-commenter bot commented Jan 12, 2025 • edited Loading

Benchmarks

darccio commented Jan 17, 2025

rarguelloF Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

rarguelloF Jan 22, 2025

Choose a reason for hiding this comment

rarguelloF left a comment

Choose a reason for hiding this comment

datadog-datadog-prod-us1 bot commented Jan 28, 2025 • edited Loading

Datadog Report

keisku commented Jan 12, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot commented Jan 12, 2025 •

edited

Loading

pr-commenter bot commented Jan 12, 2025 •

edited

Loading

rarguelloF Jan 22, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot commented Jan 28, 2025 •

edited

Loading