otelkit: Allow specifying a custom span type

[jackwilsdon]

This is useful for when an endpoint may be wrapping a call to another service, or when an endpoint is used as a consumer (e.g. with https://github.com/alebabai/go-kit-kafka). The go-kit opentracing provider supports this via use of WithTags.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: jackwilsdon / name: Jack Wilsdon (46389fe, 4eb938c)

[pellared]

The span kind is strictly defined by the HTTP semantic conventions. See: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.21.0/specification/trace/semantic_conventions/http.md#http-server-semantic-conventions

If you want to change the semantic conventions then you should create an issue (and maybe PR) in https://github.com/open-telemetry/semantic-conventions (new repo for semantic conventions).

I think this PR should be closed or at least moved to draft.

[jackwilsdon]

go-kit supports transports other than HTTP (e.g. it comes with a AMQP transport) - https://github.com/open-telemetry/opentelemetry-specification/blob/4229197f45f0da0642bc7c0127e84c8aa1439489/specification/trace/semantic_conventions/messaging.md#span-kind mentions using the consumer/producer span kinds and otelkit doesn't seem to be specific to the HTTP transport.

[codecov]

Codecov Report

Merging #4044 (416a83f) into main (9437f9b) will increase coverage by 0.0%.
The diff coverage is 100.0%.

❗ Current head 416a83f differs from pull request most recent head 4eb938c. Consider uploading reports for the commit 4eb938c to get more accurate results

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #4044   +/-   ##
=====================================
  Coverage   79.2%   79.2%           
=====================================
  Files        165     165           
  Lines      10318   10324    +6     
=====================================
+ Hits        8173    8179    +6     
  Misses      2008    2008           
  Partials     137     137           

Impacted Files	Coverage Δ
...umentation/github.com/go-kit/kit/otelkit/config.go	100.0% <100.0%> (ø)
...entation/github.com/go-kit/kit/otelkit/endpoint.go	83.0% <100.0%> (+0.5%)	⬆️

[pellared]

Second review after getting more feedback.

[pellared]

I think that only two values of trace.SpanKind are valid:

• trace.SpanKindServer
• trace.SpanKindConsumer

Can we change and add WithSpanKindServer() and WithSpanKindConsumer() options instead?

Credits to @Aneurysm9

[jackwilsdon]

I've been using SpanKindInternal on my endpoints and instead putting server/consumer on the transport's spans, as it's up to the transport whether the endpoint is called as part of a server or consumer. Changing this to just those two options somewhat limits the flexibility here.

The kit/tracing/opentracing package offers TraceEndpoint which allows using any span type.

[pellared]

I am not following. Why would you set to Internal?

as it's up to the transport whether the endpoint is called as part of a server or consumer

How would you modify the span kind? If it would be possible then you would not even need this functionality.

However, now I see that go-kit transport/endpoint can also be a "consumer" or "client" so probably it is better to keep as it is. Approving again 😉

[pellared]

I looked what the instrumentation does and it does not do any context extraction or injection, so is kind of makes internal spans... Also I do not think it applies any OTel Semantic Conventions.

I start to feel that this module should be deprecated and moved to https://github.com/go-kit/kit/tree/master/tracing 😬

dismissing

[pellared]

Closing as during SIG meeting we agreed that we are dropping support for otelkit.
See #4085

Still, thank you for your contribution ❤️