From e074d7f736124b7465d3806f8029c2b004c089c9 Mon Sep 17 00:00:00 2001 From: sivakumarsc Date: Wed, 27 Aug 2025 21:49:50 +0530 Subject: [PATCH 1/2] refactor(opentelemetry-core): simplify parsePairKeyValue() --- CHANGELOG.md | 1 + packages/opentelemetry-core/package.json | 1 + .../opentelemetry-core/src/baggage/utils.ts | 39 +++++--- .../test/performance/benchmark/index.js | 17 ++++ .../benchmark/parsePairKeyValue.js | 90 +++++++++++++++++++ 5 files changed, 134 insertions(+), 14 deletions(-) create mode 100644 packages/opentelemetry-core/test/performance/benchmark/index.js create mode 100644 packages/opentelemetry-core/test/performance/benchmark/parsePairKeyValue.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 604a48bf10c..2876884f7dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2 * refactor: replace assertRejects() with assert.rejects() [#5614](https://github.com/open-telemetry/opentelemetry-js/pull/5614) @cjihrig * refactor(core): migrate from deprecated semconv constants [#5575](https://github.com/open-telemetry/opentelemetry-js/pull/5575) @alumni55748 * refactor(opentelemetry-core): simplify `parseKeyPairsIntoRecord()` [#5610](https://github.com/open-telemetry/opentelemetry-js/pull/5610) @cjihrig +* refactor(opentelemetry-core): simplify `parsePairKeyValue()` [#5885](https://github.com/open-telemetry/opentelemetry-js/pull/5885) @sivakumarsc ## 2.0.0 diff --git a/packages/opentelemetry-core/package.json b/packages/opentelemetry-core/package.json index 194715d3585..b80128c684a 100644 --- a/packages/opentelemetry-core/package.json +++ b/packages/opentelemetry-core/package.json @@ -28,6 +28,7 @@ "watch": "tsc --build --watch tsconfig.json tsconfig.esm.json tsconfig.esnext.json", "precompile": "lerna run version --scope @opentelemetry/core --include-dependencies", "prewatch": "npm run precompile", + "test:bench": "node test/performance/benchmark/index.js", "peer-api-check": "node ../../scripts/peer-api-check.js", "align-api-deps": "node ../../scripts/align-api-deps.js" }, diff --git a/packages/opentelemetry-core/src/baggage/utils.ts b/packages/opentelemetry-core/src/baggage/utils.ts index e96a3cc10b9..d3d010e6f89 100644 --- a/packages/opentelemetry-core/src/baggage/utils.ts +++ b/packages/opentelemetry-core/src/baggage/utils.ts @@ -57,24 +57,35 @@ export function getKeyPairs(baggage: Baggage): string[] { export function parsePairKeyValue( entry: string ): ParsedBaggageKeyValue | undefined { - const valueProps = entry.split(BAGGAGE_PROPERTIES_SEPARATOR); - if (valueProps.length <= 0) return; - const keyPairPart = valueProps.shift(); - if (!keyPairPart) return; + if (!entry) return; + const metadataSeparatorIndex = entry.indexOf(BAGGAGE_PROPERTIES_SEPARATOR); + const keyPairPart = metadataSeparatorIndex === -1 + ? entry + : entry.substring(0, metadataSeparatorIndex); + const separatorIndex = keyPairPart.indexOf(BAGGAGE_KEY_PAIR_SEPARATOR); if (separatorIndex <= 0) return; - const key = decodeURIComponent( - keyPairPart.substring(0, separatorIndex).trim() - ); - const value = decodeURIComponent( - keyPairPart.substring(separatorIndex + 1).trim() - ); + + const rawKey = keyPairPart.substring(0, separatorIndex).trim(); + const rawValue = keyPairPart.substring(separatorIndex + 1).trim(); + + if (!rawKey || !rawValue) return; + + let key: string; + let value: string; + try { + key = decodeURIComponent(rawKey); + value = decodeURIComponent(rawValue); + } catch { + return; + } + let metadata; - if (valueProps.length > 0) { - metadata = baggageEntryMetadataFromString( - valueProps.join(BAGGAGE_PROPERTIES_SEPARATOR) - ); + if (metadataSeparatorIndex !== -1 && metadataSeparatorIndex < entry.length - 1) { + const metadataString = entry.substring(metadataSeparatorIndex + 1); + metadata = baggageEntryMetadataFromString(metadataString); } + return { key, value, metadata }; } diff --git a/packages/opentelemetry-core/test/performance/benchmark/index.js b/packages/opentelemetry-core/test/performance/benchmark/index.js new file mode 100644 index 00000000000..52bff39aee7 --- /dev/null +++ b/packages/opentelemetry-core/test/performance/benchmark/index.js @@ -0,0 +1,17 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +require('./parsePairKeyValue'); diff --git a/packages/opentelemetry-core/test/performance/benchmark/parsePairKeyValue.js b/packages/opentelemetry-core/test/performance/benchmark/parsePairKeyValue.js new file mode 100644 index 00000000000..ae6a7488425 --- /dev/null +++ b/packages/opentelemetry-core/test/performance/benchmark/parsePairKeyValue.js @@ -0,0 +1,90 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const Benchmark = require('benchmark'); +const { parsePairKeyValue } = require('../../../build/src/baggage/utils'); +const { baggageEntryMetadataFromString } = require('@opentelemetry/api'); + +// Constants +const BAGGAGE_PROPERTIES_SEPARATOR = ';'; +const BAGGAGE_KEY_PAIR_SEPARATOR = '='; + +// Old implementation for comparison +function parsePairKeyValue_Old(entry) { + const valueProps = entry.split(BAGGAGE_PROPERTIES_SEPARATOR); + if (valueProps.length <= 0) return; + const keyPairPart = valueProps.shift(); + if (!keyPairPart) return; + const separatorIndex = keyPairPart.indexOf(BAGGAGE_KEY_PAIR_SEPARATOR); + if (separatorIndex <= 0) return; + const key = decodeURIComponent( + keyPairPart.substring(0, separatorIndex).trim() + ); + const value = decodeURIComponent( + keyPairPart.substring(separatorIndex + 1).trim() + ); + let metadata; + if (valueProps.length > 0) { + metadata = baggageEntryMetadataFromString( + valueProps.join(BAGGAGE_PROPERTIES_SEPARATOR) + ); + } + return { key, value, metadata }; +} + +const suite = new Benchmark.Suite(); + +suite.on('cycle', event => { + console.log(String(event.target)); +}); + +// Simple key-value pairs +suite.add('parsePairKeyValue simple (old)', function() { + parsePairKeyValue_Old('key1=value1'); +}); + +suite.add('parsePairKeyValue simple (new)', function() { + parsePairKeyValue('key1=value1'); +}); + +// Key-value with metadata +suite.add('parsePairKeyValue with metadata (old)', function() { + parsePairKeyValue_Old('key1=value1;metadata=sample'); +}); + +suite.add('parsePairKeyValue with metadata (new)', function() { + parsePairKeyValue('key1=value1;metadata=sample'); +}); + +// URI encoded values +suite.add('parsePairKeyValue URI encoded (old)', function() { + parsePairKeyValue_Old('user%20id=john%20doe'); +}); + +suite.add('parsePairKeyValue URI encoded (new)', function() { + parsePairKeyValue('user%20id=john%20doe'); +}); + +// Complex case +suite.add('parsePairKeyValue complex (old)', function() { + parsePairKeyValue_Old('user%20id=john%20doe;metadata=user%20info;tenant=prod'); +}); + +suite.add('parsePairKeyValue complex (new)', function() { + parsePairKeyValue('user%20id=john%20doe;metadata=user%20info;tenant=prod'); +}); + +suite.run(); From 9de69b8bdd2e8cd97d573dcabc519072a26454d1 Mon Sep 17 00:00:00 2001 From: sivakumarsc Date: Fri, 5 Sep 2025 11:35:21 +0530 Subject: [PATCH 2/2] fix lint --- packages/opentelemetry-core/src/baggage/utils.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/opentelemetry-core/src/baggage/utils.ts b/packages/opentelemetry-core/src/baggage/utils.ts index d3d010e6f89..6346e8c8771 100644 --- a/packages/opentelemetry-core/src/baggage/utils.ts +++ b/packages/opentelemetry-core/src/baggage/utils.ts @@ -59,9 +59,10 @@ export function parsePairKeyValue( ): ParsedBaggageKeyValue | undefined { if (!entry) return; const metadataSeparatorIndex = entry.indexOf(BAGGAGE_PROPERTIES_SEPARATOR); - const keyPairPart = metadataSeparatorIndex === -1 - ? entry - : entry.substring(0, metadataSeparatorIndex); + const keyPairPart = + metadataSeparatorIndex === -1 + ? entry + : entry.substring(0, metadataSeparatorIndex); const separatorIndex = keyPairPart.indexOf(BAGGAGE_KEY_PAIR_SEPARATOR); if (separatorIndex <= 0) return; @@ -70,7 +71,6 @@ export function parsePairKeyValue( const rawValue = keyPairPart.substring(separatorIndex + 1).trim(); if (!rawKey || !rawValue) return; - let key: string; let value: string; try { @@ -81,11 +81,14 @@ export function parsePairKeyValue( } let metadata; - if (metadataSeparatorIndex !== -1 && metadataSeparatorIndex < entry.length - 1) { + if ( + metadataSeparatorIndex !== -1 && + metadataSeparatorIndex < entry.length - 1 + ) { const metadataString = entry.substring(metadataSeparatorIndex + 1); metadata = baggageEntryMetadataFromString(metadataString); } - + return { key, value, metadata }; }