slashmo / swift-otel

OpenTelemetry client built for server-side Swift
https://swiftpackageindex.com/slashmo/swift-otel
Apache License 2.0
69 stars 22 forks source link

Provide a Swift Metrics backend that exports metrics using OTLP/gRPC #84

Open simonjbeaumont opened 8 months ago

simonjbeaumont commented 8 months ago

This repo currently provides a Swift Tracing backend that exports spans using OTLP, but currently does not provide a backend for Swift Metrics or Swift Log.

I'd like to use this issue to capture and agree a design for how this package can provide a backend for Swift Metrics.

While the Swift Tracing and OTel Tracing APIs are a very close match, there's not such a great impedance match between the Swift Metrics and OTel Metrics APIs. Specifically, the Swift Metrics API is a subset of what can be expressed using OTel Metrics. Additionally the OTel specification is very principled about the API and implementation that language SDK packages should provide. Furthermore, there is already another package in the OpenTelemetry project which is attempting to provide such an SDK for Swift.

We've discussed this offline and, for these reasons, we think the most pragmatic approach is for this package to not try to provide a "full-blown" OTel SDK for metrics, but rather provide a Swift Metrics backend that exports metrics over OTLP/gRPC, such that it can be used with an OTel Collector or other receivers that accept OTLP.

Since the Swift Metrics API the simpler of the two APIs—e.g. there is no choice of aggregation temporality— so it will require simpler backing types to implement. We can even leverage almost identical backing types used in other Swift Metrics backends: notably Swift Prometheus.

We can then provide the mapping from the metric state to the subset of the OTLP datamodel required when exporting the metrics. For example, a Counter will always emit an OTLP Sum with cumulative aggregation temporality (cf. delta).

We will use a principled layered approach similar to the tracing support in this package and decouple the storage types from both the Swift Metrics backend support and the OTLP exporting. For the OTLP exporter we will try to use terms and configuration that map to the OTel specification to the extent it makes sense with the subset of functionality we're providing.

Rough breakdown

Work-in-progress branch

For those that want to follow along as the work progresses, I am maintaining a WIP branch, which I will keep rebasing against main as PRs land, and so will show the current state of outstanding items. This can be used to see how these types compose, including how they are used in the example projects: https://github.com/slashmo/swift-otel/compare/main...simonjbeaumont:sb/metrics-wip.

Outstanding questions

slashmo commented 8 months ago

the most pragmatic approach is for this package to not try to provide a "full-blown" OTel SDK for metrics, but rather provide a Swift Metrics backend that exports metrics over OTLP/gRPC

+1 on this implementation plan, it's very similar to how I approached tracing support in this library. swift-otel is server-side Swift first, OTel second.

For the OTLP exporter we will try to use terms and configuration that map to the OTel specification to the extent it makes sense with the subset of functionality we're providing.

Also +1 on this, no need to reinvent defaults and/or environment variable names for configuration.

ktoso commented 8 months ago

Thanks for the writeup, that all sounds great :)

Let's have this package be minimal, and we should have a chat with swift-opentelemetry as well at some point to see how we can help them improve the server focus in that implementation 👍

simonjbeaumont commented 7 months ago

OK, I'd like to tackle one of the open questions to unblock progress; this one: How should we handle unit and description with metrics created through the Swift Metrics API, which currently only provides a set of labels?

It's probably clear just from the question, but to spell it out: OTLP data points have a description and unit field. Both of these are considered identifying in the OTel spec. The spec also says that these are optional in the API/SDK. Because of this, we're technically providing compliant OTLP metrics by doing nothing and emitting empty values for these fields in the export, but we need to decide if/how we let users set these.

Given that, at the time of metric recording, Swift Metrics API users only have a set of (String, String) labels, I can see two options:

  1. We elevate the semantics of labels with special keys. Specifically, we will use the labels with keys description and unit for the description and unit in the OTLP export respectively.
  2. We have some method of configuring this at the time of bootstrap; some sort of dictionary from metric name to these units and descriptions.

I see (2) as a non-starter because the user bootstrapping the metrics system isn't necessarily the user recording the metrics (e.g. if using a dependency that records metrics through the Swift Metrics API), so strikes me to have limited value and present a layering violation.

@slashmo @ktoso @franzbusch WDYT to (1). Let's park any discussion of whether Swift Metrics API could evolve for the time being.

ktoso commented 7 months ago

Tbh 1) sounds totally reasonable, since otel has some well known fields, and we can match the name... this sounds reasonable. Agree that 2) mixes things up and ("who knows what") as you said.

FranzBusch commented 7 months ago

I agree 1) seems reasonable and if a user puts a description or unit label into the metric then we should just use that for OTEL.

simonjbeaumont commented 7 months ago

@Joannis @ktoso @slashmo and I met and hashed out some answers to the open questions. Concrete actions, which I will add as subtasks to this issue.