update the OTLP Exporter to match the changes in the metrics specifications (labels -> attributes) #300
Conversation
vvydier
requested review from
SergeyKanzhelev,
nachoBonafonte and
bryce-b
as code owners
|
|
|
This should resolve #293 |
| @@ -54,114 +54,132 @@ struct MetricsAdapter { | |||
| guard let gaugeData = $0 as? SumData<Double> else { | |||
| break | |||
| } | |||
| var protoDataPoint = Opentelemetry_Proto_Metrics_V1_DoubleDataPoint() | |||
| // var protoDataPoint = Opentelemetry_Proto_Metrics_V1_DoubleDataPoint() | |||
There was a problem hiding this comment.
This can be removed I think
There was a problem hiding this comment.
Thanks Trevor for catching it, I am cleaning up the commented sections of the code, as I was changing the MetricsAdapter.
| gaugeData.labels.forEach { | ||
| var kvp = Opentelemetry_Proto_Common_V1_StringKeyValue() | ||
| // var kvp = Opentelemetry_Proto_Common_V1_StringKeyValue() |
There was a problem hiding this comment.
This can be removed I think
| case .intGauge: | ||
| guard let gaugeData = $0 as? SumData<Int> else { | ||
| break | ||
| } | ||
|
|
||
| var protoDataPoint = Opentelemetry_Proto_Metrics_V1_IntDataPoint() | ||
| // var protoDataPoint = Opentelemetry_Proto_Metrics_V1_IntDataPoint() |
There was a problem hiding this comment.
This can be removed I think
| protoDataPoint.timeUnixNano = gaugeData.timestamp.timeIntervalSince1970.toNanoseconds | ||
| protoDataPoint.startTimeUnixNano = gaugeData.startTimestamp.timeIntervalSince1970.toNanoseconds | ||
| gaugeData.labels.forEach { | ||
| var kvp = Opentelemetry_Proto_Common_V1_StringKeyValue() | ||
| // var kvp = Opentelemetry_Proto_Common_V1_StringKeyValue() |
There was a problem hiding this comment.
This can be removed I think
| case .doubleSum: | ||
| guard let sumData = $0 as? SumData<Double> else { | ||
| break | ||
| } | ||
|
|
||
| var protoDataPoint = Opentelemetry_Proto_Metrics_V1_DoubleDataPoint() | ||
| protoDataPoint.value = sumData.sum | ||
| // var protoDataPoint = Opentelemetry_Proto_Metrics_V1_DoubleDataPoint() |
There was a problem hiding this comment.
This can be removed I think
| case .doubleSummary: | ||
|
|
||
| guard let summaryData = $0 as? SummaryData<Double> else { | ||
| break | ||
| } | ||
| var protoDataPoint = Opentelemetry_Proto_Metrics_V1_DoubleSummaryDataPoint() | ||
| // var protoDataPoint = Opentelemetry_Proto_Metrics_V1_DoubleSummaryDataPoint() |
There was a problem hiding this comment.
This can be removed I think
| case .intSum: | ||
| guard let sumData = $0 as? SumData<Int> else { | ||
| break | ||
| } | ||
| var protoDataPoint = Opentelemetry_Proto_Metrics_V1_IntDataPoint() | ||
| protoDataPoint.value = Int64(sumData.sum) | ||
| // var protoDataPoint = Opentelemetry_Proto_Metrics_V1_IntDataPoint() |
There was a problem hiding this comment.
This can be removed I think
| protoDataPoint.timeUnixNano = sumData.timestamp.timeIntervalSince1970.toNanoseconds | ||
| protoDataPoint.startTimeUnixNano = sumData.startTimestamp.timeIntervalSince1970.toNanoseconds | ||
| sumData.labels.forEach { | ||
| var kvp = Opentelemetry_Proto_Common_V1_StringKeyValue() | ||
| // var kvp = Opentelemetry_Proto_Common_V1_StringKeyValue() | ||
| var kvp = Opentelemetry_Proto_Common_V1_KeyValue() |
There was a problem hiding this comment.
This can be removed I think
| case .intSummary: | ||
| guard let summaryData = $0 as? SummaryData<Int> else { | ||
| break | ||
| } | ||
| var protoDataPoint = Opentelemetry_Proto_Metrics_V1_DoubleSummaryDataPoint() | ||
| // var protoDataPoint = Opentelemetry_Proto_Metrics_V1_DoubleSummaryDataPoint() |
There was a problem hiding this comment.
This can be removed I think
| case .intHistogram: | ||
| guard let histogramData = $0 as? HistogramData<Int> else { | ||
| break | ||
| } | ||
| var protoDataPoint = Opentelemetry_Proto_Metrics_V1_DoubleHistogramDataPoint() | ||
| // var protoDataPoint = Opentelemetry_Proto_Metrics_V1_DoubleHistogramDataPoint() |
There was a problem hiding this comment.
This can be removed I think
Codecov Report
@@ Coverage Diff @@
## main #300 +/- ##
==========================================
- Coverage 75.35% 74.45% -0.90%
==========================================
Files 361 361
Lines 22569 22796 +227
==========================================
- Hits 17006 16972 -34
- Misses 5563 5824 +261
Continue to review full report at Codecov.
|
|
I tested this in my environment and it's working great, thanks! |
|
@vvydier any updates on this? Is this just waiting on CLA? |
|
Yes, waiting on CNCF to resolve the CLA issue. I have opened a case and they are helping to resolve the issue. I will let you as soon as its resolved, the build and test should go through after that. Thanks |
|
The problem was that I had the GitHub user.email set to my work email when I committed. I may just re-do this in another PR to get past the EasyCLA issue! |
|
No issues at all, please create another PR |
|
/easycla |
labels has been deprecated in otel-proto and attributes are now used in it's place. This was reported by @trevor-dialpad and the OTLP Exporter needed this change if a user uses opentelemetry-swift with along other otel sdk's or uses it with newer version of the otel-collector