Repository navigation
Conversation
AbhiPrasad
left a comment
There was a problem hiding this comment.
can we add some benchmarks for this?
There was a problem hiding this comment.
might be interesting to lru cache some of these, but we should benchmark!
There was a problem hiding this comment.
LRU cache what? We could probably cache the resource and scope encodings. 🤔
There was a problem hiding this comment.
not sure, I think we gotta benchmark and burn some agent tokens. Feel free to do this as a follow up
There was a problem hiding this comment.
We should benchmark comparing the also hand rolled @opentelemetry/otlp-transformer serializer package. From my quick testing, this PR has a perf deteriorate when it serializes batched small spans, comparing to @opentelemetry/otlp-transformer.
batch pr-23 otlp-transformer ratio
10 37242 44638 0.83x
100 3776 4818 0.78x
1000 378 489 0.77x
(throughput, higher means faster.)
Details
const { createInput } = require('./helpers/otlp');
const { version } = require('../package.json');
const { ProtobufTraceSerializer } = require('@opentelemetry/otlp-transformer');
// Shared by identity so the serializer puts all spans in one ResourceSpans.
const resource = {
attributes: {
'service.name': 'benchmark',
'telemetry.sdk.name': 'microtel',
'telemetry.sdk.language': 'nodejs',
'telemetry.sdk.version': version,
'process.runtime.name': 'nodejs',
'process.runtime.version': process.version,
'process.pid': process.pid,
},
};
const instrumentationScope = { name: 'microtel', version };
const context = { traceId: 'ab'.repeat(16), spanId: 'cd'.repeat(8), traceFlags: 1 };
const now = Date.now();
const hrTime = (ms) => [Math.floor(ms / 1000), (ms % 1000) * 1e6];
// Non-ASCII, so both encoders take their multi-byte UTF-8 paths.
const urlPath = (i) => `/商品/${i}/日本語-💥`;
const createOtelSpans = (batchSize) =>
Array.from({ length: batchSize }, (_, i) => ({
name: 'GET',
kind: 2, // SpanKind.CLIENT
spanContext: () => context,
startTime: hrTime(now),
endTime: hrTime(now + 10),
attributes: {
'http.request.method': 'GET',
'url.full': `https://example.com/items/${i}`,
'url.path': urlPath(i),
'server.address': 'example.com',
'server.port': 443,
'network.protocol.version': '1.1',
'http.response.status_code': 200,
},
events: [],
links: [],
status: { code: 0 },
droppedAttributesCount: 0,
droppedEventsCount: 0,
droppedLinksCount: 0,
resource,
instrumentationScope,
}));
// Median batches/s over 30 samples, after warmup.
function measure(encode, iterations) {
const rates = [];
for (let i = 0; i < 50; i++) {
const start = process.hrtime.bigint();
for (let j = 0; j < iterations; j++) encode();
rates.push((iterations * 1e9) / Number(process.hrtime.bigint() - start));
}
return rates.slice(20).sort((a, b) => a - b)[15];
}
console.log('batch microtel otlp-transformer ratio');
for (const batchSize of [10, 100, 1000]) {
const { encoder, spans } = createInput(batchSize, false);
spans.forEach((span, i) => span.setAttribute('url.path', urlPath(i)));
const otelSpans = createOtelSpans(batchSize);
const iterations = 20000 / batchSize;
const microtel = measure(() => encoder.encode(spans), iterations);
const otel = measure(
() => ProtobufTraceSerializer.serializeRequest(otelSpans),
iterations,
);
console.log(
`${String(batchSize).padStart(5)}` +
`${Math.round(microtel).toString().padStart(11)}` +
`${Math.round(otel).toString().padStart(18)}` +
`${(microtel / otel).toFixed(2).padStart(7)}x`,
);
}|
This is also a fraction of the size and written in only an hour or so. 🤷 This is not intended to be some battle-tested implementation, this is a starting point. It gives us gRPC support with the least complexity possible to make it easy to evolve. It's all internal, so we can adjust whatever parts of it we like to make it more performant in the future. |
|
The question is why do we need a new non trivial hand rolled serializer, with |
|
Because it depends on the whole ecosystem of OTel packages and we can't just pull all that in if our goal is to try to get this into Node.js core. We need something we can own. |
|
I don't think it is wise to distinguish we or them. This repo is a collaboration of Node.js and OpenTelemetry. If we found something that can be improved, the path should be paved. |
|
the less supply chain risk to node the better re: bringing in OTEL deps. I know this is a collab, but maybe we just vendor in |
@bengl may have nerd-sniped me into this. 🙈
This adds full gRPC export without any dependency on gRPC or protobuf by just directly encoding the object model of OTLP.
It’s also pretty fast, since it knows what size of buffer to allocate ahead of time. 🙂