Skip to content

Commit

Permalink
Remove InstrumentationLibrarySpans and InstrumentationLibraryMetrics
Browse files Browse the repository at this point in the history
Resolves #149
Resolves #148

# Problem

## Traces

The traces proto currently contains InstrumentationLibrarySpans which does
not have clearly defined semantics. InstrumentationLibrarySpans may contain a
number of spans all associated with an InstrumentationLibrary. The nature of
this association is not clear.

The InstrumentationLibrary has a name and a version. It is not clear if
these fields are part of Resource identity or are attributes of a Span.
Presumably they should be interpreted as attributes of the Span.

I am not aware of any other trace protocols or backends that have the
equivalent of InstrumentationLibrary concept. However ultimately all span data
produced by OpenTelemetry libraries will end up in a backend and the
InstrumentationLibrary concept must be mapped to an existing concept. Span
attributes seem to be the only concept that fit the bill. Using attributes from
the start of the collection pipeline removes the need to deal with
InstrumentationLibrary by all codebases that need to make a mapping decision
(Collector, backend ingest points, etc).

To illustrate the data structure that was needed before this commit,
here is an example:

```yaml
resource_spans:
  resource:
    ...
  instrumentation_library_spans:
    - instrumentation_library:
        name: io.opentelemetry.redis
      spans:
        - name: request
            start_time: 123

    - instrumentation_library:
        name: io.opentelemetry.apache.httpd
      spans:
        - name: request
            start_time: 456
```

See below what the data structure becomes after implementing the proposed
solution.

## Metrics

The metrics proto currently includes InstrumentationLibraryMetrics which does
not have clearly defined semantics. InstrumentationLibraryMetrics may contain
a number of metrics all associated with one InstrumentationLibrary. The nature
of this association is not clear.

The InstrumentationLibrary has a name and a version. It is not clear if
these fields are part of metric identity. For example if I have 2 different
InstrumentationLibrarys each having a different name and both containing a
Metric that have the same MetricDescriptor.name are these 2 different
timeseries or the same one?

To illustrate the data structure that was needed before this commit,
here is an example:

```yaml
resource_metrics:
  resource:
    ...
  instrumentation_library_metrics:
    - instrumentation_library:
        name: io.opentelemetry.redis
      metrics:
        - metric_descriptor:
            name: request.count
          int64_data_points:
            - value: 10

    - instrumentation_library:
        name: io.opentelemetry.apache.httpd
      metrics:
        - metric_descriptor:
            name: request.count
          int64_data_points:
            - value: 200
```

See below what the data structure becomes after implementing the proposed
solution.

# Solution

## Traces

This commit removes `InstrumentationLibrarySpans` message type from the protocol.
We will add semantic conventions for recording instrumentation library in Span
attributes.

The benefits of this approach over using `InstrumentationLibrarySpans` are the following:

- There is not need for a new concept and new message type at the protocol
  level. This adds unnecessary complexity to all codebases that need to read and
  write traces but don't care about instrumentation library concept (likely the
  majory of codebases).

- It uses the general concept of attributes that already exists and is well
  understood and by doing so makes the semantics of instrumentation library name
  clear.

After removing `InstrumentationLibrarySpans` concept we have this data structure:

```yaml
resource_spans:
  resource:
    ...
  spans:
    - name: request
      start_time: 123
      attributes:
        - key: instrumentation.library.name
          value: io.opentelemetry.redis

    - name: request
      start_time: 456
      attributes:
        - key: instrumentation.library.name
          value: io.opentelemetry.apache.httpd
```

Once this commit is merged language SDKs will need to make a corresponding change
and add "instrumentation.library.name" (or whatever name is accepted in semantic
conventions) to Span attributes automatically.

## Metrics

This commit removes `InstrumentationLibraryMetrics` message type from the protocol.
We will add semantic conventions for recording instrumentation library in Span
attributes.

Semantically the name of `InstrumentationLibrary` is equivalent to a metric label
so we will use metric labels to record the library name and version.

The benefits of this approach over using `InstrumentationLibraryMetrics` are the following:

- There is not need for a new concept and new message type at the protocol
  level. This adds unnecessary complexity to all codebases that need to read and
  write metrics but don't care about instrumentation library concept (likely the
  majority of codebases).

- It uses the general concept of metric labels that already exists and is well
  understood and by doing so makes the semantics of instrumentation library name
  clear. The instrumentation library name is one of the labels that form
  timeseries identifier.

- It makes mapping to other metric protocols and backend clearer. I am not aware
  of any other metric protocol or backend that have the equivalent of
  `InstrumentationLibrary` concept. However ultimately all metric data produced
  by OpenTelemetry libraries will end up in a backend and the
  `InstrumentationLibrary` concept must be mapped to an existing concept. Labels
  seem to be the only concept that fit the bill. Using labels from the start of
  the collection pipeline removes the need to deal with `InstrumentationLibrary`
  by all codebases that need to make a mapping or translation decision
  (Collector, backend ingest points, etc).

After removing `InstrumentationLibraryMetrics` concept we have this data structure:

```yaml
resource_metrics:
  resource:
    ...
  metrics:
    - metric_descriptor:
        name: "request.count"
        int64_data_points:
          - value: 10
            labels:
              - key: instrumentation.library.name
                value: io.opentelemetry.redis

    - metric_descriptor:
        name: "request.count"
        int64_data_points:
          - value: 200
            labels:
              - key: instrumentation.library.name
                value: io.opentelemetry.apache.httpd
```
  • Loading branch information
Tigran Najaryan committed May 8, 2020
1 parent 28e2774 commit 1d0f94d
Show file tree
Hide file tree
Showing 8 changed files with 270 additions and 366 deletions.
48 changes: 48 additions & 0 deletions gen/go/collector/metrics/v1/metrics_service.pb.gw.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

48 changes: 48 additions & 0 deletions gen/go/collector/trace/v1/trace_service.pb.gw.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

97 changes: 22 additions & 75 deletions gen/go/common/v1/common.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 1d0f94d

Please sign in to comment.