elastic / apm

Elastic Application Performance Monitoring - resources and general issue tracking for Elastic APM.
https://www.elastic.co/apm
Apache License 2.0
384 stars 114 forks source link

Spec adding a span links API #620

Closed trentm closed 2 years ago

trentm commented 2 years ago

Refs: #594

Notes for reviewers

Checklist

apmmachine commented 2 years ago

:green_heart: Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

#### Build stats * Start Time: 2022-05-02T04:32:00.078+0000 * Duration: 3 min 48 sec

trentm commented 2 years ago

The following are some comments/notes on how each of the APM agent APIs could add span links support, if it is decided that is worthwhile for each language agent. Do not take these as requirements at all -- you all know the various APM agents better than I.

Node.js APM agent

Here is the OTel API:

The addition to the Node.js APM agent public API could be:

  // Link, Attributes, and `links` are intended to be compatible with OTel's
  // equivalent APIs in "opentelemetry-js-api/src/trace/link.ts", with the
  // exception that attributes are not yet accepted.
  export interface Link {
    /** A traceparent-format string, Transaction, or Span. */
    context: Transaction | Span | string; // This is a SpanContext in OTel.
  }

  export interface TransactionOptions {
    startTime?: number;
    childOf?: Transaction | Span | string;
    links?: Link[];
  }

  export interface SpanOptions {
    startTime?: number;
    childOf?: Transaction | Span | string;
    links?: Link[];
    exitSpan?: boolean;
  }

The context here mirrors the existing childOf option to startSpan(...) that encapsulates the needed link data. OTel has a SpanContext object that encapsulates this data.

Usage in JS code would look something like:

const span = apm.startSpan("my-span-name", {
  links: [{ context: record.messageAttributes.traceparent }]
})

The following was considered and rejected: Minimally the API could be reduced to the following. However, that means supporting two different calling forms if/when attributes are supported, which is unnecessarily complexity for a rarely used option.

const span = apm.startSpan("my-span-name", {
  links: [record.messageAttributes.traceparent]
})
trentm commented 2 years ago

Python APM agent

In opentelemetry-python/../src/opentelemetry/trace/__init__.py the start span API is:

    @abstractmethod
    def start_span(
        self,
        name: str,
        context: Optional[Context] = None,
        links: _Links = None,
        ...
    ) -> "Span":

    @contextmanager
    @abstractmethod
    def start_as_current_span(
        self,
        name: str,
        context: Optional[Context] = None,
        links: _Links = None,
        ...
    ) -> Iterator["Span"]:

# where `_Links` is basically (modulo some typings) a list of:

class Link:
    def __init__(self, context, attributes):
        # ...

The Python APM agent could perhaps add a similar links argument to elasticapm.capture_span and elasticapm.async_capture_span. Also to Client.begin_transaction()?

trentm commented 2 years ago

Go APM agent

Presumably a links could be added to SpanOptions (and TransactionOptions?) similar to OTel's SpanConfig, though the Link struct would differ.

type SpanOptions struct {
  // ...
  links      []Link
}