contrib/graphql: use naming schema #1926

rarguelloF · 2023-04-19T15:40:41Z

What does this PR do?

Implement naming schema for graphql integrations:

99designs/gqlgen
- Note about v1: graphql.query and graphql.mutation spans become graphql.server.request.
graph-gophers/graphql-go

Motivation

Improve service and span names.

Describe how to test/QA your changes

Reviewer's Checklist

Changed code has unit tests for its functionality.
If this interacts with the agent in a new way, a system test has been added.

pr-commenter · 2023-04-19T15:46:11Z

Benchmarks

Comparing candidate commit 7bc4d69 in PR branch rarguelloF/naming-schema-graphql with baseline commit b6486b2 in branch main.

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

dianashevchenko · 2023-04-24T12:36:43Z

contrib/99designs/gqlgen/tracer.go

+func serverSpanName(octx *graphql.OperationContext) string {
+	nameV0 := "graphql.request"
+	if octx != nil && octx.Operation != nil {
+		nameV0 = fmt.Sprintf("%s.%s", ext.SpanTypeGraphQL, octx.Operation.Operation)


QQ: so if octx.Operation != nil, octx.Operation.Operation is always filled ? just checking 👀

I'm not sure as this depends on how it's 99designs/gqlgen is implemented but I would say it shouldn't be empty. Also I didn't check in detail as I just kept the existing behavior here.

dianashevchenko · 2023-04-24T12:40:01Z

contrib/graph-gophers/graphql-go/graphql_test.go

 )

 type testResolver struct{}

 func (*testResolver) Hello() string                    { return "Hello, world!" }
 func (*testResolver) HelloNonTrivial() (string, error) { return "Hello, world!", nil }

+const testServerSchema = `


Do we need this specifically here as a constant as it is used only once right below?

I mainly did this change for readibility, so anyone writing tests using the newTestServer function has better visibility on what is the used schema.

dianashevchenko · 2023-04-24T15:30:40Z

contrib/graph-gophers/graphql-go/graphql_test.go

+		assert.Equal(t, "graphql.field", spans[0].OperationName())
+		assert.Equal(t, "graphql.server.request", spans[1].OperationName())
+	}
+	ddService := namingschematest.TestDDService


Used once, but if helps readability - leave it

dianashevchenko · 2023-04-24T15:32:56Z

contrib/graph-gophers/graphql-go/graphql_test.go

@@ -210,3 +186,40 @@ func TestAnalyticsSettings(t *testing.T) {
 		assertRate(t, mt, 0.23, WithAnalyticsRate(0.23))
 	})
 }
+
+func TestNamingSchema(t *testing.T) {
+	genSpans := namingschematest.GenSpansFn(func(t *testing.T, serviceOverride string) []mocktracer.Span {


So why again is serviceOverride a string, and not a bool?

I think it could be a bool but it would require changes in many other packages. I'll deal with this issue in the refactor PR I'm preparing after all these are merged 😄

…ch as queuename tags contrib: upgrade labstack/echo/v4 from v4.2.0 to v4.9.0 (#1891) ci: fix flaky lint job (#1892) contrib/elasticsearch: use naming schema (#1897) ci: introduce golangci (#1898) appsec: suspicious request blocking (#1797) Co-authored-by: Julio Guerra <julio@datadog.com> ci/golangci-lint: skip google.golang.org/grpc.v12 (#1899) .github/workflows: run ASM and RC system-tests scenarios (#1900) contrib/hashicorp/vault: use naming schema (#1868) contrib/database/sql: add WithIgnoreQueryTypes option (#1823) Co-authored-by: Zarir Hamza <zarir.hamza@datadoghq.com> Co-authored-by: Rodrigo Argüello <rodrigo.arguello@datadoghq.com> contrib/database/sql: use naming schema (#1895) internal/appsec: add server.request.method address (#1893) Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com> Co-authored-by: François Mazeau <francois.mazeau@datadoghq.com> internal/appsec/dyngo: atomic instrumentation swapping (#1873) Co-authored-by: François Mazeau <francois.mazeau@datadoghq.com> go.mod: datadog-agent/pkg/remoteconfig/state 7.45.0-rc.1 (#1902) internal/version: bump to v1.51.0 (#1912) ddtrace/tracer: don't set empty tracestate propagation tag (#1910) go.mod: github.com/DataDog/datadog-agent/pkg/obfuscate 7.45.0-rc.1 (#1916) appsec: add blocking SDK body operation (#1901) * Modifying the appsec api: adding appsec.MonitorParsedHTTPBody an error as return value * Adding a call to the WAF to check for security event synchronously with a call to appsec.MonitorParsedHTTPBody on the body passed as parameter * Removing the call to the WAF done on the body an the end of a request because we moved it. * Refactoring the waf addresses storage and access Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com> ddtrace/{opentelemetry,opentracer}: add telemetry (#1909) internal/appsec: fix user ID event detection (#1918) internal/telemetry: track tracer init time metric (#1896) Co-authored-by: Andrew Glaude <andrew.glaude@datadoghq.com> internal/appsec/remoteconfig: fix rules overrides (#1921) contrib/mongodb: use naming schema (#1908) contrib/syndtr/goleveldb/leveldb: use naming schema (#1914) contrib/tidwall/buntdb: use naming schema (#1913) internal/appsec: do not ignore the appsec events rate limiter (#1927) remoteconfig: remove empty products and don't override appsec rules data (#1925) contrib/kafka: refactor tests (#1907) contrib/google.golang.org/grpc: use naming schema (#1919) contrib/twitchtv/twirp: use naming schema (#1920) contrib/http: use naming schema (#1929) ddtrace/tracer: reset decision maker during fallback behavior of w3c header extraction (#1933) contrib/cassandra: use naming schema (#1911) Co-authored-by: Diana Shevchenko <40775148+dianashevchenko@users.noreply.github.com> contrib/redis: use naming schema (#1906) Co-authored-by: Andrew Glaude <andrew.glaude@datadoghq.com> ci/system-tests: more scenarios with parallel jobs (#1938) ci: update linter job and add bodyclose (#1942) contrib/redis/go-redis.v9: support v9 (#1730) Add support for new go-redis version v9. It does 2 things: Copy existing version 8 files to a new path, /redis/go-redis.v9. Make changes to support version 9. Fixes #1710 format and rerun go tidy get rid of prints add topLevelRegion assertions remove confusing named return values and todo comment ddtrace/tracer: ensure access to trace tags is concurrency-safe (#1948) Spancontext marshaling was accessing tracer internal structures without a lock, resulting in a data race and panic. This commit adds a few methods to trace to allow safe access to the tags and propagatingTags members of trace to the marshaling code. Fixes #1944 ddtrace/tracer: mark context updated when SetUser is called (#1949) Fixes a minor logic mistake when setting a user on a span lint and add default switch case refactor resourceNameKey and value assignments restructure functions to be left aligned use internal logger, be less verbose with function names go back to normal switch type and format Set keyTraceID128 on first span in the chunk only (#1946) go.mod: upgrade go-libddwaf to v1.2.0 (#1953) Co-authored-by: Julio Guerra <julio@datadog.com> contrib/database/sql: fix bug where options were always overwritten by register options (#1904) Co-authored-by: Diana Shevchenko <40775148+dianashevchenko@users.noreply.github.com> ci/smoke-tests: update the go.sum file after go get -u (#1957) contrib/net/http: don't set empty string values as span tags (#1956) Do not set span fields when they are not configured so the tracer can put the defaults in. use normal string then derefence rever go.mod and go.sum changes contrib/internal/httptrace: remove naming schema from init (#1960) contrib/graphql: use naming schema (#1926) internal/telemetry: trim the dependencies version prefix v (#1963) contrib/aws: use naming schema (#1931) contrib/cloud.google.com/go/pubsub.v1: use naming schema (#1937) go mod tidy lint and fix test

rarguelloF added 4 commits April 18, 2023 18:14

contrib/google.golang.org/grpc: use naming schema

01f5cbf

Merge branch 'main' into rarguelloF/naming-schema-grpc

b7efd90

Merge branch 'main' into rarguelloF/naming-schema-grpc

a2e8eb1

contrib/graphql: use naming schema

a612ee8

rarguelloF changed the base branch from main to rarguelloF/naming-schema-grpc April 19, 2023 15:41

rarguelloF marked this pull request as ready for review April 19, 2023 15:42

rarguelloF requested a review from a team April 19, 2023 15:42

rarguelloF requested a review from a team as a code owner April 19, 2023 15:42

Base automatically changed from rarguelloF/naming-schema-grpc to main April 21, 2023 09:11

rarguelloF added 4 commits April 21, 2023 14:10

Merge branch 'main' into rarguelloF/naming-schema-graphql

c3f9cac

add missing function

187a4b2

fix function name

fbc1a0b

Merge branch 'main' into rarguelloF/naming-schema-graphql

19d8bb2

dianashevchenko reviewed Apr 24, 2023

View reviewed changes

rarguelloF requested a review from dianashevchenko April 27, 2023 18:25

rarguelloF added 2 commits April 28, 2023 16:29

Merge branch 'main' into rarguelloF/naming-schema-graphql

6a59147

fix linter

cb9c5fb

dianashevchenko approved these changes May 5, 2023

View reviewed changes

rarguelloF added 2 commits May 5, 2023 14:48

Merge branch 'main' into rarguelloF/naming-schema-graphql

e2b4382

Merge branch 'main' into rarguelloF/naming-schema-graphql

7bc4d69

rarguelloF merged commit 7155f12 into main May 5, 2023

rarguelloF deleted the rarguelloF/naming-schema-graphql branch May 5, 2023 13:31

katiehockman pushed a commit that referenced this pull request Jun 6, 2023

contrib/graphql: use naming schema (#1926)

7293677

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

contrib/graphql: use naming schema #1926

contrib/graphql: use naming schema #1926

rarguelloF commented Apr 19, 2023 •

edited

Loading

pr-commenter bot commented Apr 19, 2023 •

edited

Loading

dianashevchenko Apr 24, 2023

rarguelloF Apr 27, 2023

dianashevchenko Apr 24, 2023

rarguelloF Apr 27, 2023

dianashevchenko Apr 24, 2023

dianashevchenko Apr 24, 2023

rarguelloF Apr 27, 2023

contrib/graphql: use naming schema #1926

contrib/graphql: use naming schema #1926

Conversation

rarguelloF commented Apr 19, 2023 • edited Loading

What does this PR do?

Motivation

Describe how to test/QA your changes

Reviewer's Checklist

pr-commenter bot commented Apr 19, 2023 • edited Loading

Benchmarks

dianashevchenko Apr 24, 2023

Choose a reason for hiding this comment

rarguelloF Apr 27, 2023

Choose a reason for hiding this comment

dianashevchenko Apr 24, 2023

Choose a reason for hiding this comment

rarguelloF Apr 27, 2023

Choose a reason for hiding this comment

dianashevchenko Apr 24, 2023

Choose a reason for hiding this comment

dianashevchenko Apr 24, 2023

Choose a reason for hiding this comment

rarguelloF Apr 27, 2023

Choose a reason for hiding this comment

rarguelloF commented Apr 19, 2023 •

edited

Loading

pr-commenter bot commented Apr 19, 2023 •

edited

Loading