Add 99designs/gqlgen instrumentation

[ravilushqa]

This PR adds tracer for 99designs/gqlgen

[ravilushqa]

hey @Aneurysm9 @dashpole, @evantorrie, @jmacd, @MrAlias, @paivagustavo @XSAM this is available for review now.
Please let me know if you have any comments. Waiting for your feedback

[hirenvadalia]

Any plan to merge this?

[ravilushqa]

@hirenvadalia im still waiting for review

[jmandel1027]

This is awesome @ravilushqa! we'd love to see this merged so we can start leveraging this ourselves

[codecov]

Codecov Report

Merging #761 (031a377) into main (4fa9762) will increase coverage by 0.5%.
The diff coverage is 88.1%.

❗ Current head 031a377 differs from pull request most recent head 4d64960. Consider uploading reports for the commit 4d64960 to get more accurate results

@@           Coverage Diff           @@
##            main    #761     +/-   ##
=======================================
+ Coverage   69.6%   70.1%   +0.5%     
=======================================
  Files         77      80      +3     
  Lines       4955    5099    +144     
=======================================
+ Hits        3451    3578    +127     
- Misses      1365    1378     +13     
- Partials     139     143      +4     

Impacted Files	Coverage Δ
...ion/github.com/99designs/gqlgen/otelgqlgen/tags.go	78.5% <78.5%> (ø)
...n/github.com/99designs/gqlgen/otelgqlgen/gqlgen.go	91.3% <91.3%> (ø)
...n/github.com/99designs/gqlgen/otelgqlgen/config.go	100.0% <100.0%> (ø)

[hobbsh]

First of all, awesome PR! I have a couple questions - possibly out of scope:

• How do you see users integrating this with their logging? We've only been able to retrieve the traceID/spanID from within the middleware itself and log that way, which means we would have to fork this to use it.
• How could we make use of exemplars (traceID in prometheus metrics) with this?

[ravilushqa]

@hobbsh thanks!

How do you see users integrating this with their logging? We've only been able to retrieve the traceID/spanID from within the middleware itself and log that way, which means we would have to fork this to use it.

If I understand correctly u talk about data from gql. In that situation i see using logging middleware like in my PR if it's needed. If u need data from my middleware, it's stored in spans, so u can catch it from it.

How could we make use of exemplars (traceID in prometheus metrics) with this?

I didn't work with exemplars so it's hard to say for me.

[hobbsh]

If I understand correctly u talk about data from gql. In that situation i see using logging middleware like in my PR if it's needed. If u need data from my middleware, it's stored in spans, so u can catch it from it.

The use-case is to log the traceID with the GraphQL query so we can correlate logs to traces. I am not seeing an example of using logging middleware like zap (or otherwise) in your PR to do this.

[ravilushqa]

The use-case is to log the traceID with the GraphQL query so we can correlate logs to traces. I am not seeing an example of using logging middleware like zap (or otherwise) in your PR to do this.

this pr has nothing to do with logging, so there is no such example.
In your use-case u can create trace in middleware. Trace will be stored in context.
For zap u can use  enrich logs with traceID by getting it from context.
for example:
logger.With(zap.String("trace-id", trace.SpanContextFromContext(ctx).TraceID().String())
in addition u can use zap.Hooks to enrich spans by adding log info to spans that getting from ctx too.

[hobbsh]

The use-case is to log the traceID with the GraphQL query so we can correlate logs to traces. I am not seeing an example of using logging middleware like zap (or otherwise) in your PR to do this.

this pr has nothing to do with logging, so there is no such example.
In your use-case u can create trace in middleware. Trace will be stored in context.
For zap u can use  enrich logs with traceID by getting it from context.
for example:
logger.With(zap.String("trace-id", trace.SpanContextFromContext(ctx).TraceID().String())
in addition u can use zap.Hooks to enrich spans by adding log info from spans that getting from ctx too.

I appreciate you taking the time to describe this. Thanks!

[pellared]

PS. I do not have any more findings 😉

[pellared]

@ravilushqa Nice work. I will try to ask others to review. In meantime, you can create a dedicated repository that will contain this instrumentation similarly to https://github.com/XSAM/otelsql

[ravilushqa]

@ravilushqa Nice work. I will try to ask others to review. In meantime, you can create a dedicated repository that will contain this instrumentation similarly to https://github.com/XSAM/otelsql

thanks! i'll work on package like that.

In addition i've found that we forget to remove serviceName from pr. i did it in the last commit

upd.
dedicated repo is here
https://github.com/ravilushqa/otelgqlgen

[nasushkov]

Hi @pellared any news when I can be merged?

[pellared]

I do not think it will be merged. See: #1100

However, I hope that my code reviews were helpful.

[MrAlias]

Thanks for the contribution. According to our new instrumentation policy, we are no longer accepting new instrumentation to the project as we do not have the developer bandwidth to support it. If you are able, please consider maintaining this instrumentation in your own repository and listing it in the the OpenTelemetry Registry.