Span interface is too wide

[nothingmuch]

To allow for more orthogonal composability, I think the Span interface should be refactored into an embedding of a number of narrower interfaces, each pertaining to different use cases (e.g. , and another one for deprecated functions.

Many applications already have context like objects, which might be easier to extend to become workable spans than to wrap in this generic type, and certainly provide more type safety than runtime errors or noop implementations would give. This would be especially helpful when such code also ties into existing logging/metrics gathering, as it allow gradual porting of homebrew stuff to something more standardized layer by layer, and would also permit the possibility of defined tags/baggage where appropriate.

[yurishkuro]

I thought about it before, but the only sensible thing I can see being worth pulling out is the logging methods. Below is the full list of span methods today:

Context() SpanContext 
Tracer() Tracer

Finish()
FinishWithOptions(opts FinishOptions)

SetOperationName(operationName string) Span

SetBaggageItem(key, val string) Span
BaggageItem(key string) string

SetTag(key string, value interface{}) Span

LogFields(fields ...log.Field)
LogKV(keyVals ...interface{})
LogEvent(event string)
LogEventWithPayload(event string, payload interface{})
Log(data LogData)

[nothingmuch]

It's more than just logging. For my use case tags and baggage setting doesn't apply, as all those concepts already exist (edit to clarify: deriving the set of tag/baggage data from our structures is easy, supporting mutation would be messy verbose and unecessary, but and noop or panicking implementations is an unfortunate way to workaround that). Secondly there is redundancy (e.g. Finish can be defined in terms of FinishWithOptions, so implementors could only do that). Finally, logging is orthogonal (at least in our use case I want to reuse the context shuffling code, and the injection/extraction code while integrating it with our existing span concept, which has higher type specificity for all areas of overlap (so it can easily fulfill the read only aspects of this interface, but not the mutating ones). The redundancy between the deprecated log interfaces and between the finish methods can also be factored out into reusable decorators.

[bhs]

@nothingmuch You're focusing on the needs of OpenTracing implementors, which is understandable because you're trying to implement the interfaces. :) That said, there are numerous places where we had to trade off an API direction that would be easier for a caller to understand/use and an API direction that would be easier for an implementor to understand/implement, and in approximately ~all of those situations we focused on the needs of the caller.

If you have concrete, specific suggestions about how to make the calling interface better, please make them, but I should forewarn you that they will be judged almost entirely by how they look to the caller (rather than implementor) of the API.

[Zaba505]

@nothingmuch @yurishkuro I've just encountered this exact situation and I'm leaning towards the following solution:

// More or less take @yurishkuro's approach but extend it with exported functions
// that mimic the interface methods
type Span interface {
  Finish()
}

type Baggager interface {
  // Don't have to return Span but keep it consistent with old
  SetBaggageItem(k, v string) Span
  BaggageItem(k string) string
}

// Provide a exported API for working with Span
func SetBaggageItem(s Span, k, v string)  Span {
  b, ok := s.(Baggager)
  if !ok {
    // If interface isn't implemented default to noop behaviour,
    // since a Noop suite is already provided i.e. tracing is an optional behaviour
    return s
  }
  b.SetBaggageItem(k, v)
  return s
}
func BaggageItem(s Span, k string) string { return "" }

This design would then be repeated for the other Span methods.

@bhs From the perspective of the client all calls are the same given Go methods are nothing but syntactic sugar for function calls, where the first arg is the method receiver. That said their code would technically be broken and I'm not 100% certain if go fix would auto rewrite these.

Any thoughts would be greatly appreciated!