Remove @_spi(Metrics) annotation from metrics API #110
Conversation
simonjbeaumont
force-pushed
the
sb/metrics-remove-spi-annotation
branch
from
3ee242a
to
60268c0
Compare
| public enum OTelAggregationTemporailty: Equatable, Sendable { | ||
| case delta | ||
| case cumulative | ||
| } |
There was a problem hiding this comment.
Discuss: do we need to wrap these enums in a struct for API reasons?
There was a problem hiding this comment.
Yeah, I think we should, doesn't hurt to do it.
There was a problem hiding this comment.
OK, I've replaced the each of the public enum types in the datamodel with a package enum wrapped in a public struct. I also removed one of them because we weren't exposing anything useful in the API (the flags).
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
==========================================
- Coverage 98.97% 98.96% -0.01%
==========================================
Files 67 67
Lines 1951 1939 -12
==========================================
- Hits 1931 1919 -12
Misses 20 20 ☔ View full report in Codecov by Sentry. |
| extension Opentelemetry_Proto_Common_V1_AnyValue { | ||
| package init(_ string: String) { | ||
| value = .stringValue(string) | ||
| } | ||
| } | ||
|
|
||
| @_spi(Metrics) | ||
| extension Opentelemetry_Proto_Metrics_V1_Exemplar { | ||
| package init(_ exemplar: OTelExemplar) { | ||
| // TODO: |
There was a problem hiding this comment.
Do we want to link to an issue here?
There was a problem hiding this comment.
I've just revisited the section of the OTel spec and, at time of writing, it appears that this is still experimental1 so I'm gonna remove it entirely for now.
Footnotes
Sources/OTel/Metrics/OTelMetricExporter/OTelNoOpMetricExporter.swift
Outdated
Show resolved
Hide resolved
| public enum OTelAggregationTemporailty: Equatable, Sendable { | ||
| case delta | ||
| case cumulative | ||
| } |
There was a problem hiding this comment.
Yeah, I think we should, doesn't hurt to do it.
Sources/OTel/Metrics/OTelMetricProducer/OTLPMetricsDataModel.swift
Outdated
Show resolved
Hide resolved
simonjbeaumont
force-pushed
the
sb/metrics-remove-spi-annotation
branch
from
60268c0
to
b91be3b
Compare
| extension Opentelemetry_Proto_Common_V1_AnyValue { | ||
| package init(_ string: String) { | ||
| value = .stringValue(string) | ||
| } | ||
| } | ||
|
|
||
| @_spi(Metrics) | ||
| extension Opentelemetry_Proto_Metrics_V1_Exemplar { | ||
| package init(_ exemplar: OTelExemplar) { | ||
| // TODO: |
There was a problem hiding this comment.
I've just revisited the section of the OTel spec and, at time of writing, it appears that this is still experimental1 so I'm gonna remove it entirely for now.
Footnotes
Sources/OTel/Metrics/OTelMetricExporter/OTelNoOpMetricExporter.swift
Outdated
Show resolved
Hide resolved
Sources/OTel/Metrics/OTelMetricProducer/OTLPMetricsDataModel.swift
Outdated
Show resolved
Hide resolved
| public enum OTelAggregationTemporailty: Equatable, Sendable { | ||
| case delta | ||
| case cumulative | ||
| } |
There was a problem hiding this comment.
OK, I've replaced the each of the public enum types in the datamodel with a package enum wrapped in a public struct. I also removed one of them because we weren't exposing anything useful in the API (the flags).
simonjbeaumont
changed the title
|
//cc @ktoso any final last words before we make the API public? |
Motivation
During development of the OLTP backend for Swift Metrics (see #84), the API had been annotated with
@_spi(Metrics)until the API was finalised and to keep the main branch release-able. The shape of the API has now been finalised with input from other maintainers and it's time to bikeshed any naming and remove the SPI annotation, which is the purpose of this PR.Modifications
_@spi(Metrics)annotation from all public metrics API surface.OTelExemplar; it's still experimental in the latest revision of the OTel specification.OTelNumberDataPoint.Flags; we weren't exposing values so it serves no purpose currently.NoOpMetricExporter; it's not serving any purpose as a global default and is equivalent to just not bootstrapping the Swift Metrics with a backend.OTelAggregationTemporailty->OTelAggregationTemporality.Opentelemetry_Proto_Metrics_V1_ResourceMetrics.init(_:)(not API).Result
The API surface for metrics is publicly available.