opentelemetry-go icon indicating copy to clipboard operation
opentelemetry-go copied to clipboard

Add Name() to Span interface or Attach the name field to the context

Open holdno opened this issue 1 year ago • 7 comments

Problem Statement

I'm wondering why the name attribute of a span is not attached to the context. Having the name within the context could enable us to easily print the entire trace information in some straightforward scenarios without relying on backend implementations. For example, by using span.SpanContext().Name(), we could recursively retrieve the current trace through parent methods and log it. This would be efficient for debugging purposes.

In other words, why isn't there an interface in the current Span definition to retrieve the name of the span, while the actual implementation provides a Name() string method?

Proposed Solution

To enhance the Span interface, there are two potential approaches:

  • Add a Name() string method to the Span interface to retrieve the name of the current Span.
  • Attach the name field to the context.
// current span interface
type Span interface {
	// End completes the Span. The Span is considered complete and ready to be
	// delivered through the rest of the telemetry pipeline after this method
	// is called. Therefore, updates to the Span are not allowed after this
	// method has been called.
	End(options ...SpanEndOption)

	// AddEvent adds an event with the provided name and options.
	AddEvent(name string, options ...EventOption)

	// IsRecording returns the recording state of the Span. It will return
	// true if the Span is active and events can be recorded.
	IsRecording() bool

	// RecordError will record err as an exception span event for this span. An
	// additional call to SetStatus is required if the Status of the Span should
	// be set to Error, as this method does not change the Span status. If this
	// span is not being recorded or err is nil then this method does nothing.
	RecordError(err error, options ...EventOption)

	// SpanContext returns the SpanContext of the Span. The returned SpanContext
	// is usable even after the End method has been called for the Span.
	SpanContext() SpanContext

	// SetStatus sets the status of the Span in the form of a code and a
	// description, provided the status hasn't already been set to a higher
	// value before (OK > Error > Unset). The description is only included in a
	// status when the code is for an error.
	SetStatus(code codes.Code, description string)

	// SetName sets the Span name.
	SetName(name string)

	// SetAttributes sets kv as attributes of the Span. If a key from kv
	// already exists for an attribute of the Span it will be overwritten with
	// the value contained in kv.
	SetAttributes(kv ...attribute.KeyValue)

	// TracerProvider returns a TracerProvider that can be used to generate
	// additional Spans on the same telemetry pipeline as the current Span.
	TracerProvider() TracerProvider
}

Thanks.

holdno avatar Oct 07 '23 07:10 holdno

The span interface is specified for all languages, in the specification. There may be discussions in the spec PRs about the decision not to be able to read the name. https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#span

Adding a method to the span interface isn't possible without a change in the specification. And since that API is stable, it would require a major version bump. So it's extremely unlikely to happen on its own.

dmathieu avatar Oct 07 '23 09:10 dmathieu

Yes, the specification explicitly defines the components of a Span, with the first item being the name. However, why is it not exposed in the interface definition for Go, while the concrete implementation includes it, such as Name() string?

// Name returns the name of this span.
func (s *recordingSpan) Name() string {
	s.mu.Lock()
	defer s.mu.Unlock()
	return s.name
}

holdno avatar Oct 07 '23 10:10 holdno

Yes, the specification explicitly defines the components of a Span, with the first item being the name. However, why is it not exposed in the interface definition for Go, while the concrete implementation includes it, such as Name() string?

// Name returns the name of this span.
func (s *recordingSpan) Name() string {
	s.mu.Lock()
	defer s.mu.Unlock()
	return s.name
}

Please provide a link where the specification defines the name method of the span interface that you are proposing to add to this projects interface.

MrAlias avatar Oct 09 '23 14:10 MrAlias

The specification defines that a span must have a name. Since name is a required field, why can't it be accessed externally?

Link: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#span

A Span represents a single operation within a trace. Spans can be nested to form a trace tree. Each trace contains a root span, which typically describes the entire operation and, optionally, one or more sub-spans for its sub-operations.

Spans encapsulate:

- The span name

I'm not quite sure what issues the current Span interface and ReadWriteSpan interface are trying to address. Why not directly provide ReadWriteSpan to users? Although I can perform conversions through assertions.

holdno avatar Oct 09 '23 16:10 holdno

The span has a name property defined in the specification as you linked above. However, as @damemi pointed out, it does not define the accessor method you are proposing here. We do not plan to add additions to the span interface without the specification including this first.

MrAlias avatar Oct 09 '23 16:10 MrAlias

How can I raise this issue with the specification?

holdno avatar Oct 09 '23 16:10 holdno

@holdno see https://github.com/open-telemetry/opentelemetry-specification/blob/main/CONTRIBUTING.md#proposing-a-change

Smaller changes can follow a shorter process: [...]

pellared avatar Oct 10 '23 06:10 pellared