From 629fa5034a4e5791fbeeff6aa695affad727372b Mon Sep 17 00:00:00 2001 From: davidwitten Date: Wed, 15 Jul 2020 23:34:57 +0000 Subject: [PATCH 1/8] Changed metrics to functions and added summary --- .../src/transformMetrics.ts | 44 +++- .../test/common/transformMetrics.test.ts | 79 ++++-- .../test/helper.ts | 226 +++++++++++------- 3 files changed, 248 insertions(+), 101 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/transformMetrics.ts b/packages/opentelemetry-exporter-collector/src/transformMetrics.ts index be14cea32a..c5b829abcf 100644 --- a/packages/opentelemetry-exporter-collector/src/transformMetrics.ts +++ b/packages/opentelemetry-exporter-collector/src/transformMetrics.ts @@ -60,6 +60,9 @@ export function toCollectorType( if (metric.aggregator instanceof HistogramAggregator) { return opentelemetryProto.metrics.v1.MetricDescriptorType.HISTOGRAM; } + if (metric.aggregator instanceof MinMaxLastSumCountAggregator) { + return opentelemetryProto.metrics.v1.MetricDescriptorType.SUMMARY; + } if (metric.descriptor.valueType == api.ValueType.INT) { return opentelemetryProto.metrics.v1.MetricDescriptorType.INT64; } @@ -132,10 +135,7 @@ export function toSingularPoint( timeUnixNano: number; value: number; } { - const pointValue = - metric.aggregator instanceof MinMaxLastSumCountAggregator - ? (metric.aggregator.toPoint().value as Distribution).last - : (metric.aggregator.toPoint().value as number); + const pointValue = metric.aggregator.toPoint().value as number; return { labels: toCollectorLabels(metric.labels), @@ -172,6 +172,31 @@ export function toHistogramPoint( }; } +/** + * Returns a SummaryPoint to the collector + * @param metric + * @param startTime + */ +export function toSummaryPoint( + metric: MetricRecord, + startTime: number +): opentelemetryProto.metrics.v1.SummaryDataPoint { + const distValue = metric.aggregator.toPoint().value as Distribution; + return { + labels: toCollectorLabels(metric.labels), + sum: distValue.sum, + count: distValue.count, + startTimeUnixNano: startTime, + timeUnixNano: core.hrTimeToNanoseconds( + metric.aggregator.toPoint().timestamp + ), + percentileValues: [ + { percentile: 0, value: distValue.min }, + { percentile: 100, value: distValue.max }, + ], + }; +} + /** * Converts a metric to be compatible with the collector * @param metric @@ -190,6 +215,15 @@ export function toCollectorMetric( histogramDataPoints: [toHistogramPoint(metric, startTime)], }; } + if ( + toCollectorType(metric) === + opentelemetryProto.metrics.v1.MetricDescriptorType.SUMMARY + ) { + return { + metricDescriptor: toCollectorMetricDescriptor(metric), + summaryDataPoints: [toSummaryPoint(metric, startTime)], + }; + } if (metric.descriptor.valueType == api.ValueType.INT) { return { metricDescriptor: toCollectorMetricDescriptor(metric), @@ -201,7 +235,7 @@ export function toCollectorMetric( metricDescriptor: toCollectorMetricDescriptor(metric), doubleDataPoints: [toSingularPoint(metric, startTime)], }; - } // TODO: Add support for summary points once implemented + } return { metricDescriptor: toCollectorMetricDescriptor(metric), diff --git a/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts b/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts index 6e4563e6b6..8b123a0b51 100644 --- a/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts @@ -17,46 +17,95 @@ import * as assert from 'assert'; import * as transform from '../../src/transformMetrics'; import { mockCounter, + mockDoubleCounter, mockObserver, mockedResources, mockedInstrumentationLibraries, multiResourceMetrics, multiInstrumentationLibraryMetrics, ensureCounterIsCorrect, + ensureDoubleCounterIsCorrect, ensureObserverIsCorrect, mockHistogram, ensureHistogramIsCorrect, ensureValueRecorderIsCorrect, mockValueRecorder, } from '../helper'; -import { HistogramAggregator } from '@opentelemetry/metrics'; +import { + HistogramAggregator, + MetricRecord, + SumAggregator, +} from '@opentelemetry/metrics'; import { hrTimeToNanoseconds } from '@opentelemetry/core'; +import { Resource } from '@opentelemetry/resources'; describe('transformMetrics', () => { describe('toCollectorMetric', () => { + const counter: MetricRecord = mockCounter(); + const doubleCounter: MetricRecord = mockDoubleCounter(); + const observer: MetricRecord = mockObserver(); + const histogram: MetricRecord = mockHistogram(); + const recorder: MetricRecord = mockValueRecorder(); + const invalidMetric: MetricRecord = { + descriptor: { + name: 'name', + description: 'description', + unit: 'unit', + metricKind: 8, // Not a valid metricKind + valueType: 2, // Not double or int + }, + labels: {}, + aggregator: new SumAggregator(), + resource: new Resource({}), + instrumentationLibrary: { name: 'x', version: 'y' }, + }; + beforeEach(() => { + // Counter + counter.aggregator.update(1); + + // Double Counter + doubleCounter.aggregator.update(8); + + // Observer + observer.aggregator.update(3); + observer.aggregator.update(6); + + // Histogram + histogram.aggregator.update(7); + histogram.aggregator.update(14); + (histogram.aggregator as HistogramAggregator).reset(); + + // ValueRecorder + recorder.aggregator.update(5); + }); it('should convert metric', () => { - mockCounter.aggregator.update(1); ensureCounterIsCorrect( - transform.toCollectorMetric(mockCounter, 1592602232694000000), - hrTimeToNanoseconds(mockCounter.aggregator.toPoint().timestamp) + transform.toCollectorMetric(counter, 1592602232694000000), + hrTimeToNanoseconds(counter.aggregator.toPoint().timestamp) ); - mockObserver.aggregator.update(10); ensureObserverIsCorrect( - transform.toCollectorMetric(mockObserver, 1592602232694000000), - hrTimeToNanoseconds(mockObserver.aggregator.toPoint().timestamp) + transform.toCollectorMetric(observer, 1592602232694000000), + hrTimeToNanoseconds(observer.aggregator.toPoint().timestamp) ); - mockHistogram.aggregator.update(7); - mockHistogram.aggregator.update(14); - (mockHistogram.aggregator as HistogramAggregator).reset(); ensureHistogramIsCorrect( - transform.toCollectorMetric(mockHistogram, 1592602232694000000), - hrTimeToNanoseconds(mockHistogram.aggregator.toPoint().timestamp) + transform.toCollectorMetric(histogram, 1592602232694000000), + hrTimeToNanoseconds(histogram.aggregator.toPoint().timestamp) ); - mockValueRecorder.aggregator.update(5); ensureValueRecorderIsCorrect( - transform.toCollectorMetric(mockValueRecorder, 1592602232694000000), - hrTimeToNanoseconds(mockValueRecorder.aggregator.toPoint().timestamp) + transform.toCollectorMetric(recorder, 1592602232694000000), + hrTimeToNanoseconds(recorder.aggregator.toPoint().timestamp) + ); + + ensureDoubleCounterIsCorrect( + transform.toCollectorMetric(doubleCounter, 1592602232694000000), + hrTimeToNanoseconds(doubleCounter.aggregator.toPoint().timestamp) + ); + + const emptyMetric = transform.toCollectorMetric( + invalidMetric, + 1592602232694000000 ); + assert.deepStrictEqual(emptyMetric.int64DataPoints, []); }); }); describe('toCollectorMetricDescriptor', () => { diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index f2ed9cf048..edf4e766c1 100644 --- a/packages/opentelemetry-exporter-collector/test/helper.ts +++ b/packages/opentelemetry-exporter-collector/test/helper.ts @@ -59,77 +59,105 @@ const traceIdArr = [ const spanIdArr = [94, 16, 114, 97, 246, 79, 165, 62]; const parentIdArr = [120, 168, 145, 80, 152, 134, 67, 136]; -export const mockCounter: MetricRecord = { - descriptor: { - name: 'test-counter', - description: 'sample counter description', - unit: '1', - metricKind: MetricKind.COUNTER, - valueType: ValueType.INT, - }, - labels: {}, - aggregator: new SumAggregator(), - resource: new Resource({ - service: 'ui', - version: 1, - cost: 112.12, - }), - instrumentationLibrary: { name: 'default', version: '0.0.1' }, -}; +export function mockCounter(): MetricRecord { + return { + descriptor: { + name: 'test-counter', + description: 'sample counter description', + unit: '1', + metricKind: MetricKind.COUNTER, + valueType: ValueType.INT, + }, + labels: {}, + aggregator: new SumAggregator(), + resource: new Resource({ + service: 'ui', + version: 1, + cost: 112.12, + }), + instrumentationLibrary: { name: 'default', version: '0.0.1' }, + }; +} -export const mockObserver: MetricRecord = { - descriptor: { - name: 'test-observer', - description: 'sample observer description', - unit: '2', - metricKind: MetricKind.VALUE_OBSERVER, - valueType: ValueType.DOUBLE, - }, - labels: {}, - aggregator: new MinMaxLastSumCountAggregator(), - resource: new Resource({ - service: 'ui', - version: 1, - cost: 112.12, - }), - instrumentationLibrary: { name: 'default', version: '0.0.1' }, -}; +export function mockDoubleCounter(): MetricRecord { + return { + descriptor: { + name: 'test-counter', + description: 'sample counter description', + unit: '1', + metricKind: MetricKind.COUNTER, + valueType: ValueType.DOUBLE, + }, + labels: {}, + aggregator: new SumAggregator(), + resource: new Resource({ + service: 'ui', + version: 1, + cost: 112.12, + }), + instrumentationLibrary: { name: 'default', version: '0.0.1' }, + }; +} -export const mockValueRecorder: MetricRecord = { - descriptor: { - name: 'test-recorder', - description: 'sample recorder description', - unit: '3', - metricKind: MetricKind.VALUE_RECORDER, - valueType: ValueType.INT, - }, - labels: {}, - aggregator: new MinMaxLastSumCountAggregator(), - resource: new Resource({ - service: 'ui', - version: 1, - cost: 112.12, - }), - instrumentationLibrary: { name: 'default', version: '0.0.1' }, -}; +export function mockObserver(): MetricRecord { + return { + descriptor: { + name: 'test-observer', + description: 'sample observer description', + unit: '2', + metricKind: MetricKind.VALUE_OBSERVER, + valueType: ValueType.DOUBLE, + }, + labels: {}, + aggregator: new MinMaxLastSumCountAggregator(), + resource: new Resource({ + service: 'ui', + version: 1, + cost: 112.12, + }), + instrumentationLibrary: { name: 'default', version: '0.0.1' }, + }; +} -export const mockHistogram: MetricRecord = { - descriptor: { - name: 'test-hist', - description: 'sample observer description', - unit: '2', - metricKind: MetricKind.VALUE_OBSERVER, - valueType: ValueType.DOUBLE, - }, - labels: {}, - aggregator: new HistogramAggregator([10, 20]), - resource: new Resource({ - service: 'ui', - version: 1, - cost: 112.12, - }), - instrumentationLibrary: { name: 'default', version: '0.0.1' }, -}; +export function mockValueRecorder(): MetricRecord { + return { + descriptor: { + name: 'test-recorder', + description: 'sample recorder description', + unit: '3', + metricKind: MetricKind.VALUE_RECORDER, + valueType: ValueType.INT, + }, + labels: {}, + aggregator: new MinMaxLastSumCountAggregator(), + resource: new Resource({ + service: 'ui', + version: 1, + cost: 112.12, + }), + instrumentationLibrary: { name: 'default', version: '0.0.1' }, + }; +} + +export function mockHistogram(): MetricRecord { + return { + descriptor: { + name: 'test-hist', + description: 'sample observer description', + unit: '2', + metricKind: MetricKind.VALUE_OBSERVER, + valueType: ValueType.DOUBLE, + }, + labels: {}, + aggregator: new HistogramAggregator([10, 20]), + resource: new Resource({ + service: 'ui', + version: 1, + cost: 112.12, + }), + instrumentationLibrary: { name: 'default', version: '0.0.1' }, + }; +} const traceIdBase64 = 'HxAI3I4nDoXECg18OTmyeA=='; const spanIdBase64 = 'XhByYfZPpT4='; @@ -285,17 +313,17 @@ export const multiResourceTrace: ReadableSpan[] = [ export const multiResourceMetrics: MetricRecord[] = [ { - ...mockCounter, + ...mockCounter(), resource: mockedResources[0], instrumentationLibrary: mockedInstrumentationLibraries[0], }, { - ...mockObserver, + ...mockObserver(), resource: mockedResources[1], instrumentationLibrary: mockedInstrumentationLibraries[0], }, { - ...mockCounter, + ...mockCounter(), resource: mockedResources[0], instrumentationLibrary: mockedInstrumentationLibraries[0], }, @@ -303,17 +331,17 @@ export const multiResourceMetrics: MetricRecord[] = [ export const multiInstrumentationLibraryMetrics: MetricRecord[] = [ { - ...mockCounter, + ...mockCounter(), resource: mockedResources[0], instrumentationLibrary: mockedInstrumentationLibraries[0], }, { - ...mockObserver, + ...mockObserver(), resource: mockedResources[0], instrumentationLibrary: mockedInstrumentationLibraries[1], }, { - ...mockCounter, + ...mockCounter(), resource: mockedResources[0], instrumentationLibrary: mockedInstrumentationLibraries[0], }, @@ -685,6 +713,29 @@ export function ensureCounterIsCorrect( }); } +export function ensureDoubleCounterIsCorrect( + metric: collectorTypes.opentelemetryProto.metrics.v1.Metric, + time: number +) { + assert.deepStrictEqual(metric, { + metricDescriptor: { + name: 'test-counter', + description: 'sample counter description', + unit: '1', + type: 4, + temporality: 3, + }, + doubleDataPoints: [ + { + labels: [], + value: 8, + startTimeUnixNano: 1592602232694000000, + timeUnixNano: time, + }, + ], + }); +} + export function ensureObserverIsCorrect( metric: collectorTypes.opentelemetryProto.metrics.v1.Metric, time: number @@ -694,15 +745,23 @@ export function ensureObserverIsCorrect( name: 'test-observer', description: 'sample observer description', unit: '2', - type: 3, + type: 6, temporality: 2, }, - doubleDataPoints: [ + summaryDataPoints: [ { - labels: [], - value: 10, startTimeUnixNano: 1592602232694000000, timeUnixNano: time, + count: 2, + sum: 9, + labels: [], + percentileValues: [ + { + percentile: 0, + value: 3, + }, + { percentile: 100, value: 6 }, + ], }, ], }); @@ -717,13 +776,18 @@ export function ensureValueRecorderIsCorrect( name: 'test-recorder', description: 'sample recorder description', unit: '3', - type: 1, + type: 6, temporality: 2, }, - int64DataPoints: [ + summaryDataPoints: [ { + count: 1, + sum: 5, labels: [], - value: 5, + percentileValues: [ + { percentile: 0, value: 5 }, + { percentile: 100, value: 5 }, + ], startTimeUnixNano: 1592602232694000000, timeUnixNano: time, }, From dc1a4928778758af8b2c43e84ee9e5a938ae4b15 Mon Sep 17 00:00:00 2001 From: davidwitten Date: Thu, 16 Jul 2020 15:55:05 +0000 Subject: [PATCH 2/8] Fixed tests --- .../test/common/CollectorMetricExporter.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts index 73e9ddc40e..1f9c56e5d6 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts @@ -52,8 +52,8 @@ describe('CollectorMetricExporter - common', () => { }; collectorExporter = new CollectorMetricExporter(collectorExporterConfig); metrics = []; - metrics.push(Object.assign({}, mockCounter)); - metrics.push(Object.assign({}, mockObserver)); + metrics.push(mockCounter()); + metrics.push(mockObserver()); }); afterEach(() => { From 52f1decdf27de4594aca483971cc9019dd0d7672 Mon Sep 17 00:00:00 2001 From: davidwitten Date: Sun, 19 Jul 2020 02:56:25 +0000 Subject: [PATCH 3/8] Remove extra assignment --- .../opentelemetry-exporter-collector/src/transformMetrics.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/transformMetrics.ts b/packages/opentelemetry-exporter-collector/src/transformMetrics.ts index c5b829abcf..fbcf3ebcd2 100644 --- a/packages/opentelemetry-exporter-collector/src/transformMetrics.ts +++ b/packages/opentelemetry-exporter-collector/src/transformMetrics.ts @@ -135,11 +135,9 @@ export function toSingularPoint( timeUnixNano: number; value: number; } { - const pointValue = metric.aggregator.toPoint().value as number; - return { labels: toCollectorLabels(metric.labels), - value: pointValue, + value: metric.aggregator.toPoint().value as number, startTimeUnixNano: startTime, timeUnixNano: core.hrTimeToNanoseconds( metric.aggregator.toPoint().timestamp From f8a9abf81763a451cdae676bfe93869d75cb02c1 Mon Sep 17 00:00:00 2001 From: davidwitten Date: Mon, 20 Jul 2020 15:42:40 +0000 Subject: [PATCH 4/8] Used existing enum --- .../test/helper.ts | 40 ++++++++++++++----- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index edf4e766c1..6896871d91 100644 --- a/packages/opentelemetry-exporter-collector/test/helper.ts +++ b/packages/opentelemetry-exporter-collector/test/helper.ts @@ -699,8 +699,12 @@ export function ensureCounterIsCorrect( name: 'test-counter', description: 'sample counter description', unit: '1', - type: 2, - temporality: 3, + type: + collectorTypes.opentelemetryProto.metrics.v1.MetricDescriptorType + .MONOTONIC_INT64, + temporality: + collectorTypes.opentelemetryProto.metrics.v1.MetricDescriptorTemporality + .CUMULATIVE, }, int64DataPoints: [ { @@ -722,8 +726,12 @@ export function ensureDoubleCounterIsCorrect( name: 'test-counter', description: 'sample counter description', unit: '1', - type: 4, - temporality: 3, + type: + collectorTypes.opentelemetryProto.metrics.v1.MetricDescriptorType + .MONOTONIC_DOUBLE, + temporality: + collectorTypes.opentelemetryProto.metrics.v1.MetricDescriptorTemporality + .CUMULATIVE, }, doubleDataPoints: [ { @@ -745,8 +753,12 @@ export function ensureObserverIsCorrect( name: 'test-observer', description: 'sample observer description', unit: '2', - type: 6, - temporality: 2, + type: + collectorTypes.opentelemetryProto.metrics.v1.MetricDescriptorType + .SUMMARY, + temporality: + collectorTypes.opentelemetryProto.metrics.v1.MetricDescriptorTemporality + .DELTA, }, summaryDataPoints: [ { @@ -776,8 +788,12 @@ export function ensureValueRecorderIsCorrect( name: 'test-recorder', description: 'sample recorder description', unit: '3', - type: 6, - temporality: 2, + type: + collectorTypes.opentelemetryProto.metrics.v1.MetricDescriptorType + .SUMMARY, + temporality: + collectorTypes.opentelemetryProto.metrics.v1.MetricDescriptorTemporality + .DELTA, }, summaryDataPoints: [ { @@ -804,8 +820,12 @@ export function ensureHistogramIsCorrect( name: 'test-hist', description: 'sample observer description', unit: '2', - type: 5, - temporality: 2, + type: + collectorTypes.opentelemetryProto.metrics.v1.MetricDescriptorType + .HISTOGRAM, + temporality: + collectorTypes.opentelemetryProto.metrics.v1.MetricDescriptorTemporality + .DELTA, }, histogramDataPoints: [ { From dda64b3cbb7449ee5a1d9c9b74b63abac6625cff Mon Sep 17 00:00:00 2001 From: davidwitten Date: Mon, 20 Jul 2020 18:52:45 +0000 Subject: [PATCH 5/8] Added variable --- .../src/transformMetrics.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/transformMetrics.ts b/packages/opentelemetry-exporter-collector/src/transformMetrics.ts index fbcf3ebcd2..97a62aae38 100644 --- a/packages/opentelemetry-exporter-collector/src/transformMetrics.ts +++ b/packages/opentelemetry-exporter-collector/src/transformMetrics.ts @@ -154,15 +154,14 @@ export function toHistogramPoint( metric: MetricRecord, startTime: number ): opentelemetryProto.metrics.v1.HistogramDataPoint { - const histValue = metric.aggregator.toPoint().value as Histogram; + const histPoint = metric.aggregator.toPoint(); + const histValue = histPoint.value as Histogram; return { labels: toCollectorLabels(metric.labels), sum: histValue.sum, count: histValue.count, startTimeUnixNano: startTime, - timeUnixNano: core.hrTimeToNanoseconds( - metric.aggregator.toPoint().timestamp - ), + timeUnixNano: core.hrTimeToNanoseconds(histPoint.timestamp), buckets: histValue.buckets.counts.map(count => { return { count }; }), @@ -179,15 +178,14 @@ export function toSummaryPoint( metric: MetricRecord, startTime: number ): opentelemetryProto.metrics.v1.SummaryDataPoint { - const distValue = metric.aggregator.toPoint().value as Distribution; + const summaryPoint = metric.aggregator.toPoint(); + const distValue = summaryPoint.value as Distribution; return { labels: toCollectorLabels(metric.labels), sum: distValue.sum, count: distValue.count, startTimeUnixNano: startTime, - timeUnixNano: core.hrTimeToNanoseconds( - metric.aggregator.toPoint().timestamp - ), + timeUnixNano: core.hrTimeToNanoseconds(summaryPoint.timestamp), percentileValues: [ { percentile: 0, value: distValue.min }, { percentile: 100, value: distValue.max }, From 360e5cb4d6f14cc70a689e6cde918fc05bea44d0 Mon Sep 17 00:00:00 2001 From: davidwitten Date: Tue, 21 Jul 2020 18:17:42 +0000 Subject: [PATCH 6/8] One variable --- .../src/transformMetrics.ts | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/transformMetrics.ts b/packages/opentelemetry-exporter-collector/src/transformMetrics.ts index 649e77e648..f192d117bf 100644 --- a/packages/opentelemetry-exporter-collector/src/transformMetrics.ts +++ b/packages/opentelemetry-exporter-collector/src/transformMetrics.ts @@ -28,6 +28,7 @@ import * as core from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { toCollectorResource } from './transform'; import { CollectorExporterBase } from './CollectorExporterBase'; +import { HrTime } from '@opentelemetry/api'; /** * Converts labels @@ -70,7 +71,6 @@ export function toCollectorType( return opentelemetryProto.metrics.v1.MetricDescriptorType.DOUBLE; } - // @TODO #1294: Add Summary once implemented return opentelemetryProto.metrics.v1.MetricDescriptorType.INVALID_TYPE; } @@ -154,18 +154,20 @@ export function toHistogramPoint( metric: MetricRecord, startTime: number ): opentelemetryProto.metrics.v1.HistogramDataPoint { - const histPoint = metric.aggregator.toPoint(); - const histValue = histPoint.value as Histogram; + const { value, timestamp } = metric.aggregator.toPoint() as { + value: Histogram; + timestamp: HrTime; + }; return { labels: toCollectorLabels(metric.labels), - sum: histValue.sum, - count: histValue.count, + sum: value.sum, + count: value.count, startTimeUnixNano: startTime, - timeUnixNano: core.hrTimeToNanoseconds(histPoint.timestamp), - buckets: histValue.buckets.counts.map(count => { + timeUnixNano: core.hrTimeToNanoseconds(timestamp), + buckets: value.buckets.counts.map(count => { return { count }; }), - explicitBounds: histValue.buckets.boundaries, + explicitBounds: value.buckets.boundaries, }; } @@ -178,17 +180,20 @@ export function toSummaryPoint( metric: MetricRecord, startTime: number ): opentelemetryProto.metrics.v1.SummaryDataPoint { - const summaryPoint = metric.aggregator.toPoint(); - const distValue = summaryPoint.value as Distribution; + const { value, timestamp } = metric.aggregator.toPoint() as { + value: Distribution; + timestamp: HrTime; + }; + return { labels: toCollectorLabels(metric.labels), - sum: distValue.sum, - count: distValue.count, + sum: value.sum, + count: value.count, startTimeUnixNano: startTime, - timeUnixNano: core.hrTimeToNanoseconds(summaryPoint.timestamp), + timeUnixNano: core.hrTimeToNanoseconds(timestamp), percentileValues: [ - { percentile: 0, value: distValue.min }, - { percentile: 100, value: distValue.max }, + { percentile: 0, value: value.min }, + { percentile: 100, value: value.max }, ], }; } From 99415a30b3459202635eb388e9a9f4d24d5abd6c Mon Sep 17 00:00:00 2001 From: davidwitten Date: Tue, 4 Aug 2020 19:14:07 +0000 Subject: [PATCH 7/8] fix: flaky test --- .../opentelemetry-sdk-node/test/sdk.test.ts | 101 ++++++++++-------- 1 file changed, 58 insertions(+), 43 deletions(-) diff --git a/packages/opentelemetry-sdk-node/test/sdk.test.ts b/packages/opentelemetry-sdk-node/test/sdk.test.ts index b9d464025d..76450b0617 100644 --- a/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -44,42 +44,49 @@ describe('Node SDK', () => { }); describe('Basic Registration', () => { - it('should not register any unconfigured SDK components', async () => { + it('should not register any unconfigured SDK components', done => { const sdk = new NodeSDK({ autoDetectResources: false, }); - - await sdk.start(); - - assert.ok(context['_getContextManager']() instanceof NoopContextManager); - assert.ok( - propagation['_getGlobalPropagator']() instanceof NoopHttpTextPropagator - ); - - assert.ok(trace.getTracerProvider() instanceof NoopTracerProvider); - assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); + setTimeout(async () => { + await sdk.start(); + + assert.ok( + context['_getContextManager']() instanceof NoopContextManager + ); + assert.ok( + propagation['_getGlobalPropagator']() instanceof + NoopHttpTextPropagator + ); + + assert.ok(trace.getTracerProvider() instanceof NoopTracerProvider); + assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); + return done(); + }); }); - it('should register a tracer provider if an exporter is provided', async () => { + it('should register a tracer provider if an exporter is provided', done => { const sdk = new NodeSDK({ traceExporter: new ConsoleSpanExporter(), autoDetectResources: false, }); - - await sdk.start(); - - assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); - - assert.ok( - context['_getContextManager']() instanceof AsyncHooksContextManager - ); - assert.ok( - propagation['_getGlobalPropagator']() instanceof CompositePropagator - ); - assert.ok(trace.getTracerProvider() instanceof NodeTracerProvider); + setTimeout(async () => { + await sdk.start(); + + assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); + + assert.ok( + context['_getContextManager']() instanceof AsyncHooksContextManager + ); + assert.ok( + propagation['_getGlobalPropagator']() instanceof CompositePropagator + ); + assert.ok(trace.getTracerProvider() instanceof NodeTracerProvider); + return done(); + }); }); - it('should register a tracer provider if a span processor is provided', async () => { + it('should register a tracer provider if a span processor is provided', done => { const exporter = new ConsoleSpanExporter(); const spanProcessor = new SimpleSpanProcessor(exporter); @@ -88,37 +95,45 @@ describe('Node SDK', () => { autoDetectResources: false, }); - await sdk.start(); + setTimeout(async () => { + await sdk.start(); - assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); + assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); - assert.ok( - context['_getContextManager']() instanceof AsyncHooksContextManager - ); - assert.ok( - propagation['_getGlobalPropagator']() instanceof CompositePropagator - ); - assert.ok(trace.getTracerProvider() instanceof NodeTracerProvider); + assert.ok( + context['_getContextManager']() instanceof AsyncHooksContextManager + ); + assert.ok( + propagation['_getGlobalPropagator']() instanceof CompositePropagator + ); + assert.ok(trace.getTracerProvider() instanceof NodeTracerProvider); + return done(); + }); }); - it('should register a meter provider if an exporter is provided', async () => { + it('should register a meter provider if an exporter is provided', done => { const exporter = new ConsoleMetricExporter(); const sdk = new NodeSDK({ metricExporter: exporter, autoDetectResources: false, }); + setTimeout(async () => { + await sdk.start(); - await sdk.start(); + assert.ok( + context['_getContextManager']() instanceof NoopContextManager + ); + assert.ok( + propagation['_getGlobalPropagator']() instanceof + NoopHttpTextPropagator + ); - assert.ok(context['_getContextManager']() instanceof NoopContextManager); - assert.ok( - propagation['_getGlobalPropagator']() instanceof NoopHttpTextPropagator - ); + assert.ok(trace.getTracerProvider() instanceof NoopTracerProvider); - assert.ok(trace.getTracerProvider() instanceof NoopTracerProvider); - - assert.ok(metrics.getMeterProvider() instanceof MeterProvider); + assert.ok(metrics.getMeterProvider() instanceof MeterProvider); + return done(); + }); }); }); }); From b8eda861db1202785cb57ce7a9decd55d7b4b8e9 Mon Sep 17 00:00:00 2001 From: davidwitten Date: Tue, 4 Aug 2020 20:19:42 +0000 Subject: [PATCH 8/8] fix: reverted test changes --- .../opentelemetry-sdk-node/test/sdk.test.ts | 101 ++++++++---------- 1 file changed, 43 insertions(+), 58 deletions(-) diff --git a/packages/opentelemetry-sdk-node/test/sdk.test.ts b/packages/opentelemetry-sdk-node/test/sdk.test.ts index 76450b0617..b9d464025d 100644 --- a/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -44,49 +44,42 @@ describe('Node SDK', () => { }); describe('Basic Registration', () => { - it('should not register any unconfigured SDK components', done => { + it('should not register any unconfigured SDK components', async () => { const sdk = new NodeSDK({ autoDetectResources: false, }); - setTimeout(async () => { - await sdk.start(); - - assert.ok( - context['_getContextManager']() instanceof NoopContextManager - ); - assert.ok( - propagation['_getGlobalPropagator']() instanceof - NoopHttpTextPropagator - ); - - assert.ok(trace.getTracerProvider() instanceof NoopTracerProvider); - assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); - return done(); - }); + + await sdk.start(); + + assert.ok(context['_getContextManager']() instanceof NoopContextManager); + assert.ok( + propagation['_getGlobalPropagator']() instanceof NoopHttpTextPropagator + ); + + assert.ok(trace.getTracerProvider() instanceof NoopTracerProvider); + assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); }); - it('should register a tracer provider if an exporter is provided', done => { + it('should register a tracer provider if an exporter is provided', async () => { const sdk = new NodeSDK({ traceExporter: new ConsoleSpanExporter(), autoDetectResources: false, }); - setTimeout(async () => { - await sdk.start(); - - assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); - - assert.ok( - context['_getContextManager']() instanceof AsyncHooksContextManager - ); - assert.ok( - propagation['_getGlobalPropagator']() instanceof CompositePropagator - ); - assert.ok(trace.getTracerProvider() instanceof NodeTracerProvider); - return done(); - }); + + await sdk.start(); + + assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); + + assert.ok( + context['_getContextManager']() instanceof AsyncHooksContextManager + ); + assert.ok( + propagation['_getGlobalPropagator']() instanceof CompositePropagator + ); + assert.ok(trace.getTracerProvider() instanceof NodeTracerProvider); }); - it('should register a tracer provider if a span processor is provided', done => { + it('should register a tracer provider if a span processor is provided', async () => { const exporter = new ConsoleSpanExporter(); const spanProcessor = new SimpleSpanProcessor(exporter); @@ -95,45 +88,37 @@ describe('Node SDK', () => { autoDetectResources: false, }); - setTimeout(async () => { - await sdk.start(); + await sdk.start(); - assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); + assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider); - assert.ok( - context['_getContextManager']() instanceof AsyncHooksContextManager - ); - assert.ok( - propagation['_getGlobalPropagator']() instanceof CompositePropagator - ); - assert.ok(trace.getTracerProvider() instanceof NodeTracerProvider); - return done(); - }); + assert.ok( + context['_getContextManager']() instanceof AsyncHooksContextManager + ); + assert.ok( + propagation['_getGlobalPropagator']() instanceof CompositePropagator + ); + assert.ok(trace.getTracerProvider() instanceof NodeTracerProvider); }); - it('should register a meter provider if an exporter is provided', done => { + it('should register a meter provider if an exporter is provided', async () => { const exporter = new ConsoleMetricExporter(); const sdk = new NodeSDK({ metricExporter: exporter, autoDetectResources: false, }); - setTimeout(async () => { - await sdk.start(); - assert.ok( - context['_getContextManager']() instanceof NoopContextManager - ); - assert.ok( - propagation['_getGlobalPropagator']() instanceof - NoopHttpTextPropagator - ); + await sdk.start(); - assert.ok(trace.getTracerProvider() instanceof NoopTracerProvider); + assert.ok(context['_getContextManager']() instanceof NoopContextManager); + assert.ok( + propagation['_getGlobalPropagator']() instanceof NoopHttpTextPropagator + ); - assert.ok(metrics.getMeterProvider() instanceof MeterProvider); - return done(); - }); + assert.ok(trace.getTracerProvider() instanceof NoopTracerProvider); + + assert.ok(metrics.getMeterProvider() instanceof MeterProvider); }); }); });