Skip to content

Commit

Permalink
fix(otlp-transformer): avoid precision loss when converting HrTime to…
Browse files Browse the repository at this point in the history 
… unix nanoseconds (#4062)

Co-authored-by: Marc Pichler <[email protected]>
  • Loading branch information
@seemk @pichlermarc
seemk and pichlermarc authored Sep 28, 2023
1 parent 6bf1b78 commit 513d067
Show file tree
Hide file tree
Showing 28 changed files with 530 additions and 420 deletions.
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ All notable changes to experimental packages in this project will be documented

### :bug: (Bug Fix)

* fix(otlp-transformer): Avoid precision loss when converting from HrTime to unix nanoseconds. [#4062](https://github.com/open-telemetry/opentelemetry-js/pull/4062)
* fix(exporter-logs-otlp-http): add @opentelemetry/api-logs as dependency

## 0.41.2
Expand Down
2 changes: 1 addition & 1 deletion experimental/packages/exporter-logs-otlp-grpc/test/logsHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export function ensureExportedLogRecordIsCorrect(logRecord: ILogRecord) {
ensureExportedAttributesAreCorrect(logRecord.attributes);
assert.strictEqual(
logRecord.timeUnixNano,
'1680253513123241728',
'1680253513123241635',
'timeUnixNano is wrong'
);
assert.strictEqual(
Expand Down
14 changes: 10 additions & 4 deletions experimental/packages/exporter-logs-otlp-http/test/logHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { Resource } from '@opentelemetry/resources';
import * as assert from 'assert';
import { VERSION } from '@opentelemetry/core';
import {
hrTimeToFixed64Nanos,
IAnyValue,
IExportLogsServiceRequest,
IKeyValue,
Expand Down Expand Up @@ -76,17 +77,22 @@ export function ensureExportedBodyIsCorrect(body?: IAnyValue) {
);
}

function hrTimeToFixed64(hrTime: HrTime) {
const { low, high } = hrTimeToFixed64Nanos(hrTime);
return { low, high };
}

export function ensureExportedLogRecordIsCorrect(logRecord: ILogRecord) {
ensureExportedBodyIsCorrect(logRecord.body);
ensureExportedAttributesAreCorrect(logRecord.attributes);
assert.strictEqual(
assert.deepStrictEqual(
logRecord.timeUnixNano,
1680253513123241700,
hrTimeToFixed64(mockedReadableLogRecord.hrTime),
'timeUnixNano is wrong'
);
assert.strictEqual(
assert.deepStrictEqual(
logRecord.observedTimeUnixNano,
1680253513123241700,
hrTimeToFixed64(mockedReadableLogRecord.hrTimeObserved),
'observedTimeUnixNano is wrong'
);
assert.strictEqual(
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/exporter-logs-otlp-proto/test/logHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ export function ensureExportedLogRecordIsCorrect(logRecord: ILogRecord) {
ensureExportedAttributesAreCorrect(logRecord.attributes);
assert.strictEqual(
logRecord.timeUnixNano,
'1680253513123241728',
'1680253513123241635',
'timeUnixNano is wrong'
);
assert.strictEqual(
logRecord.observedTimeUnixNano,
'1680253513123241728',
'1680253513123241635',
'observedTimeUnixNano is wrong'
);
assert.strictEqual(
Expand Down
20 changes: 10 additions & 10 deletions experimental/packages/exporter-trace-otlp-grpc/test/traceHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,49 +114,49 @@ export function ensureExportedEventsAreCorrect(events: IEvent[]) {
[
{
attributes: [],
timeUnixNano: '1574120165429803008',
timeUnixNano: '1574120165429803070',
name: 'fetchStart',
droppedAttributesCount: 0,
},
{
attributes: [],
timeUnixNano: '1574120165429803008',
timeUnixNano: '1574120165429803070',
name: 'domainLookupStart',
droppedAttributesCount: 0,
},
{
attributes: [],
timeUnixNano: '1574120165429803008',
timeUnixNano: '1574120165429803070',
name: 'domainLookupEnd',
droppedAttributesCount: 0,
},
{
attributes: [],
timeUnixNano: '1574120165429803008',
timeUnixNano: '1574120165429803070',
name: 'connectStart',
droppedAttributesCount: 0,
},
{
attributes: [],
timeUnixNano: '1574120165429803008',
timeUnixNano: '1574120165429803070',
name: 'connectEnd',
droppedAttributesCount: 0,
},
{
attributes: [],
timeUnixNano: '1574120165435513088',
timeUnixNano: '1574120165435513070',
name: 'requestStart',
droppedAttributesCount: 0,
},
{
attributes: [],
timeUnixNano: '1574120165436923136',
timeUnixNano: '1574120165436923070',
name: 'responseStart',
droppedAttributesCount: 0,
},
{
attributes: [],
timeUnixNano: '1574120165438688000',
timeUnixNano: '1574120165438688070',
name: 'responseEnd',
droppedAttributesCount: 0,
},
Expand Down Expand Up @@ -235,12 +235,12 @@ export function ensureExportedSpanIsCorrect(span: ISpan) {
assert.strictEqual(span.kind, 'SPAN_KIND_INTERNAL', 'kind is wrong');
assert.strictEqual(
span.startTimeUnixNano,
'1574120165429803008',
'1574120165429803070',
'startTimeUnixNano is wrong'
);
assert.strictEqual(
span.endTimeUnixNano,
'1574120165438688000',
'1574120165438688070',
'endTimeUnixNano is wrong'
);
assert.strictEqual(
Expand Down
1 change: 0 additions & 1 deletion experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,6 @@ describe('OTLPTraceExporter - node with json over http', () => {

fakeRequest.on('end', () => {
const responseBody = buff.toString();

const json = JSON.parse(responseBody) as IExportTraceServiceRequest;
const span1 = json.resourceSpans?.[0].scopeSpans?.[0].spans?.[0];
assert.ok(typeof span1 !== 'undefined', "span doesn't exist");
Expand Down
30 changes: 18 additions & 12 deletions experimental/packages/exporter-trace-otlp-http/test/traceHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
ILink,
IResource,
ISpan,
UnsignedLong,
} from '@opentelemetry/otlp-transformer';

if (typeof Buffer === 'undefined') {
Expand Down Expand Up @@ -243,54 +244,59 @@ export const multiInstrumentationLibraryTrace: ReadableSpan[] = [
},
];

function fixed64FromString(str: string) {
const { low, high } = UnsignedLong.fromString(str);
return { low, high };
}

export function ensureEventsAreCorrect(events: IEvent[]) {
assert.deepStrictEqual(
events,
[
{
timeUnixNano: 1574120165429803000,
timeUnixNano: fixed64FromString('1574120165429803070'),
name: 'fetchStart',
attributes: [],
droppedAttributesCount: 0,
},
{
timeUnixNano: 1574120165429803000,
timeUnixNano: fixed64FromString('1574120165429803070'),
name: 'domainLookupStart',
attributes: [],
droppedAttributesCount: 0,
},
{
timeUnixNano: 1574120165429803000,
timeUnixNano: fixed64FromString('1574120165429803070'),
name: 'domainLookupEnd',
attributes: [],
droppedAttributesCount: 0,
},
{
timeUnixNano: 1574120165429803000,
timeUnixNano: fixed64FromString('1574120165429803070'),
name: 'connectStart',
attributes: [],
droppedAttributesCount: 0,
},
{
timeUnixNano: 1574120165429803000,
timeUnixNano: fixed64FromString('1574120165429803070'),
name: 'connectEnd',
attributes: [],
droppedAttributesCount: 0,
},
{
timeUnixNano: 1574120165435513000,
timeUnixNano: fixed64FromString('1574120165435513070'),
name: 'requestStart',
attributes: [],
droppedAttributesCount: 0,
},
{
timeUnixNano: 1574120165436923100,
timeUnixNano: fixed64FromString('1574120165436923070'),
name: 'responseStart',
attributes: [],
droppedAttributesCount: 0,
},
{
timeUnixNano: 1574120165438688000,
timeUnixNano: fixed64FromString('1574120165438688070'),
name: 'responseEnd',
attributes: [],
droppedAttributesCount: 0,
Expand Down Expand Up @@ -364,14 +370,14 @@ export function ensureSpanIsCorrect(span: ISpan, useHex = true) {
);
assert.strictEqual(span.name, 'documentFetch', 'name is wrong');
assert.strictEqual(span.kind, ESpanKind.SPAN_KIND_INTERNAL, 'kind is wrong');
assert.strictEqual(
assert.deepStrictEqual(
span.startTimeUnixNano,
1574120165429803008,
fixed64FromString('1574120165429803070'),
'startTimeUnixNano is wrong'
);
assert.strictEqual(
assert.deepStrictEqual(
span.endTimeUnixNano,
1574120165438688000,
fixed64FromString('1574120165438688070'),
'endTimeUnixNano is wrong'
);
assert.strictEqual(
Expand Down
20 changes: 10 additions & 10 deletions experimental/packages/exporter-trace-otlp-proto/test/traceHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,42 +109,42 @@ export function ensureProtoEventsAreCorrect(events: IEvent[]) {
events,
[
{
timeUnixNano: '1574120165429803008',
timeUnixNano: '1574120165429803070',
name: 'fetchStart',
droppedAttributesCount: 0,
},
{
timeUnixNano: '1574120165429803008',
timeUnixNano: '1574120165429803070',
name: 'domainLookupStart',
droppedAttributesCount: 0,
},
{
timeUnixNano: '1574120165429803008',
timeUnixNano: '1574120165429803070',
name: 'domainLookupEnd',
droppedAttributesCount: 0,
},
{
timeUnixNano: '1574120165429803008',
timeUnixNano: '1574120165429803070',
name: 'connectStart',
droppedAttributesCount: 0,
},
{
timeUnixNano: '1574120165429803008',
timeUnixNano: '1574120165429803070',
name: 'connectEnd',
droppedAttributesCount: 0,
},
{
timeUnixNano: '1574120165435513088',
timeUnixNano: '1574120165435513070',
name: 'requestStart',
droppedAttributesCount: 0,
},
{
timeUnixNano: '1574120165436923136',
timeUnixNano: '1574120165436923070',
name: 'responseStart',
droppedAttributesCount: 0,
},
{
timeUnixNano: '1574120165438688000',
timeUnixNano: '1574120165438688070',
name: 'responseEnd',
droppedAttributesCount: 0,
},
Expand Down Expand Up @@ -219,12 +219,12 @@ export function ensureProtoSpanIsCorrect(span: ISpan) {
assert.strictEqual(span.kind, 'SPAN_KIND_INTERNAL', 'kind is wrong');
assert.strictEqual(
span.startTimeUnixNano,
'1574120165429803008',
'1574120165429803070',
'startTimeUnixNano is wrong'
);
assert.strictEqual(
span.endTimeUnixNano,
'1574120165438688000',
'1574120165438688070',
'endTimeUnixNano is wrong'
);
assert.strictEqual(
Expand Down
17 changes: 11 additions & 6 deletions ...imental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,18 +253,23 @@ const testOTLPMetricExporter = (params: TestParams) => {
exportedData[0].scopeMetrics[0].metrics[histogramIndex];
ensureExportedCounterIsCorrect(
counter,
counter.sum?.dataPoints[0].timeUnixNano,
counter.sum?.dataPoints[0].startTimeUnixNano
metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0].endTime,
metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0]
.startTime
);
ensureExportedObservableGaugeIsCorrect(
observableGauge,
observableGauge.gauge?.dataPoints[0].timeUnixNano,
observableGauge.gauge?.dataPoints[0].startTimeUnixNano
metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0]
.endTime,
metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0]
.startTime
);
ensureExportedHistogramIsCorrect(
histogram,
histogram.histogram?.dataPoints[0].timeUnixNano,
histogram.histogram?.dataPoints[0].startTimeUnixNano,
metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0]
.endTime,
metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0]
.startTime,
[0, 100],
['0', '2', '0']
);
Expand Down
Loading

1 comment on commit 513d067

@danstarns
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may have caused a breaking change going from @opentelemetry/exporter-trace-otlp-http @ 0.43.0 to 0.44.0.

We have a proxy over the top of a collector and send traces using the HTTP exporter.

Our schemas were expecting the startTimeUnixNano and endTimeUnixNano to be of type number and now they are of type object.

@seemk @pichlermarc

Please sign in to comment.