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

Concerns about high cardinality in GraphQL span names #1361

Closed
kaylareopelle opened this issue Aug 21, 2024 · 4 comments · Fixed by #1389
Closed

Concerns about high cardinality in GraphQL span names #1361

kaylareopelle opened this issue Aug 21, 2024 · 4 comments · Fixed by #1389
Assignees
Labels
area:graphql bug Something isn't working

Comments

@kaylareopelle
Copy link
Contributor

Area(s)

area:graphql

Is your change request related to a problem? Please describe.

The Ruby SIG has some concerns about the cardinality of span names for the current GraphQL semantic convention.

@karmingc opened a PR on the opentelemetry-ruby-contrib repo to update the GraphQL span names.

This would update Ruby's existing name, graphql.execute_query, to the current semantic convention's format, <graphql.operation.type> <graphql.operation.name>.

During review, @robertlaurin raised a concern about unbounded cardinality for the span names if operation name and operation type are included:

...OpenTelemetry consistently discourages high cardinality span names, and this by definition is unbounded cardinality.

GraphQL operation name is just a free form label supplied by the client caller. There's absolutely no constraints, if we don't put the raw path in an http server span name, I'm not at all convinced we should all operation name here.

Fwiw query, mutation, subscription seems reasonable on its own.
(source)

The PR is currently blocked until we can come to a resolution about the appropriate span name. 


Describe the solution you'd like

<graphql.operation.type> seems sufficient, since the <graphql.operation.name> is available in span attributes.

Describe alternatives you've considered

The GraphQL semantic convention could also be updated to more closely match the DB span name, by adding a SHOULD instead of a MUST for the name and/or add the low_cardinality caveat about db.operation.name.

The span name SHOULD be {db.operation.name} {target} if there is a (low-cardinality) {db.operation.name} available (see below for the exact definition of the {target} placeholder).
[source]

Additional context

This may be similar to #182, but since the problem focuses specifically on the span name, I thought it warranted a new issue.

@kaylareopelle kaylareopelle changed the title GraphQL span names may have unbounded cardinality? Concerns about high cardinality in GraphQL span names Aug 22, 2024
@joaopgrassi
Copy link
Member

@kaylareopelle since you folks have good ideas on how to move forward, are you opening a PR to change it?

@kaylareopelle
Copy link
Contributor Author

Hi @joaopgrassi! I can open a PR to change it! This is my first time interacting with the semantic conventions repo, so I wasn't exactly sure about the process.

@joaopgrassi
Copy link
Member

Perfect, thank you! I will assign this to you then. Feel free to reach out if you need help with anything.

@joaopgrassi joaopgrassi added bug Something isn't working and removed triage:needs-triage enhancement New feature or request labels Aug 30, 2024
@kaylareopelle
Copy link
Contributor Author

Hi @joaopgrassi! I've opened a PR for this change: #1389

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:graphql bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants