Skip to content

Commit

Permalink
feat(otlp-exporter-base): internally accept a http header provider fu…
Browse files Browse the repository at this point in the history
…nction only (#5179)
  • Loading branch information
pichlermarc authored Nov 20, 2024
1 parent a834861 commit dd5b5fb
Show file tree
Hide file tree
Showing 17 changed files with 109 additions and 80 deletions.
27 changes: 15 additions & 12 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,21 @@ All notable changes to experimental packages in this project will be documented

### :boom: Breaking Change

* feat(otlp-exporter-base)!: collapse base classes into one [#5031](https://github.com/open-telemetry/opentelemetry-js/pull/5031) @pichlermarc
* `OTLPExporterNodeBase` has been removed in favor of a platform-agnostic implementation (`OTLPExporterBase`)
* `OTLPExporterBrowserBase` has been removed in favor of a platform-agnostic implementation (`OTLPExporterBase`)
* `ExportServiceError` was intended for internal use and has been dropped from exports
* `validateAndNormalizeHeaders` was intended for internal use and has been dropped from exports
* `OTLPExporterBase` all properties are now private, the constructor now takes an `IOTLPExportDelegate`, the type parameter for config type has been dropped.
* This type is scheduled for removal in a future version of this package, please treat all exporters as `SpanExporter`, `PushMetricExporter` or `LogRecordExporter`, based on their respective type.
* feat(otlp-grpc-exporter-base)!: collapse base classes into one [#5031](https://github.com/open-telemetry/opentelemetry-js/pull/5031) @pichlermarc
* `OTLPGRPCExporterNodeBase` has been removed in favor of a platform-agnostic implementation (`OTLPExporterBase` from `@opentelemetry/otlp-exporter-base`)
* feat(otlp-exporter-base): internally accept a http header provider function only [#5179](https://github.com/open-telemetry/opentelemetry-js/pull/5179) @pichlermarc
* feat(otlp-transformer)!: accept `ResourceMetrics` in serializers instead of `ResourceMetrics[]`
* (user-facing): `ProtobufMetricsSerializer` now only accepts `ResourceMetrics` instead of `ResourceMetrics[]` to align with `PushMetricExporter` requirements
* (user-facing): `JsonMetricsSerializer` now only accepts `ResourceMetrics` instead of `ResourceMetrics[]` to align with `PushMetricExporter` requirements


### :rocket: (Enhancement)

### :bug: (Bug Fix)
Expand Down Expand Up @@ -53,18 +68,6 @@ All notable changes to experimental packages in this project will be documented
* GetFunction
* Func
* Err
* feat(otlp-exporter-base)!: collapse base classes into one [#5031](https://github.com/open-telemetry/opentelemetry-js/pull/5031)
* `OTLPExporterNodeBase` has been removed in favor of a platform-agnostic implementation (`OTLPExporterBase`)
* `OTLPExporterBrowserBase` has been removed in favor of a platform-agnostic implementation (`OTLPExporterBase`)
* `ExportServiceError` was intended for internal use and has been dropped from exports
* `validateAndNormalizeHeaders` was intended for internal use and has been dropped from exports
* `OTLPExporterBase` all properties are now private, the constructor now takes an `IOTLPExportDelegate`, the type parameter for config type has been dropped.
* This type is scheduled for removal in a future version of this package, please treat all exporters as `SpanExporter`, `PushMetricExporter` or `LogRecordExporter`, based on their respective type.
* feat(otlp-grpc-exporter-base)!: collapse base classes into one [#5031](https://github.com/open-telemetry/opentelemetry-js/pull/5031)
* `OTLPGRPCExporterNodeBase` has been removed in favor of a platform-agnostic implementation (`OTLPExporterBase` from `@opentelemetry/otlp-exporter-base`)
* feat(otlp-transformer)!: accept `ResourceMetrics` in serializers instead of `ResourceMetrics[]`
* (user-facing): `ProtobufMetricsSerializer` now only accepts `ResourceMetrics` instead of `ResourceMetrics[]` to align with `PushMetricExporter` requirements
* (user-facing): `JsonMetricsSerializer` now only accepts `ResourceMetrics` instead of `ResourceMetrics[]` to align with `PushMetricExporter` requirements

### :rocket: (Enhancement)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
OtlpHttpConfiguration,
} from './otlp-http-configuration';
import { OTLPExporterNodeConfigBase } from './legacy-node-configuration';
import { wrapStaticHeadersInFunction } from './shared-configuration';

/**
* @deprecated this will be removed in 2.0
Expand All @@ -36,7 +37,7 @@ export function convertLegacyBrowserHttpOptions(
{
url: config.url,
timeoutMillis: config.timeoutMillis,
headers: config.headers,
headers: wrapStaticHeadersInFunction(config.headers),
concurrencyLimit: config.concurrencyLimit,
},
{}, // no fallback for browser case
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { getHttpConfigurationFromEnvironment } from './otlp-http-env-configurati
import type * as http from 'http';
import type * as https from 'https';
import { diag } from '@opentelemetry/api';
import { wrapStaticHeadersInFunction } from './shared-configuration';

function convertLegacyAgentOptions(
config: OTLPExporterNodeConfigBase
Expand Down Expand Up @@ -67,7 +68,7 @@ export function convertLegacyHttpOptions(
return mergeOtlpHttpConfigurationWithDefaults(
{
url: config.url,
headers: config.headers,
headers: wrapStaticHeadersInFunction(config.headers),
concurrencyLimit: config.concurrencyLimit,
timeoutMillis: config.timeoutMillis,
compression: config.compression,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,32 +27,34 @@ import type * as https from 'https';

export interface OtlpHttpConfiguration extends OtlpSharedConfiguration {
url: string;
headers: Record<string, string>;
headers: () => Record<string, string>;
agentOptions: http.AgentOptions | https.AgentOptions;
}

function mergeHeaders(
userProvidedHeaders: Record<string, string> | undefined | null,
fallbackHeaders: Record<string, string> | undefined | null,
defaultHeaders: Record<string, string>
): Record<string, string> {
userProvidedHeaders: (() => Record<string, string>) | undefined | null,
fallbackHeaders: (() => Record<string, string>) | undefined | null,
defaultHeaders: () => Record<string, string>
): () => Record<string, string> {
const requiredHeaders = {
...defaultHeaders,
...defaultHeaders(),
};
const headers = {};

// add fallback ones first
if (fallbackHeaders != null) {
Object.assign(headers, fallbackHeaders);
}
return () => {
// add fallback ones first
if (fallbackHeaders != null) {
Object.assign(headers, fallbackHeaders());
}

// override with user-provided ones
if (userProvidedHeaders != null) {
Object.assign(headers, userProvidedHeaders);
}
// override with user-provided ones
if (userProvidedHeaders != null) {
Object.assign(headers, userProvidedHeaders());
}

// override required ones.
return Object.assign(headers, requiredHeaders);
// override required ones.
return Object.assign(headers, requiredHeaders);
};
}

function validateUserProvidedUrl(url: string | undefined): string | undefined {
Expand Down Expand Up @@ -107,7 +109,7 @@ export function getHttpConfigurationDefaults(
): OtlpHttpConfiguration {
return {
...getSharedConfigurationDefaults(),
headers: requiredHeaders,
headers: () => requiredHeaders,
url: 'http://localhost:4318/' + signalResourcePath,
agentOptions: { keepAlive: true },
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ import { baggageUtils } from '@opentelemetry/core';
import { diag } from '@opentelemetry/api';
import { getSharedConfigurationFromEnvironment } from './shared-env-configuration';
import { OtlpHttpConfiguration } from './otlp-http-configuration';
import { wrapStaticHeadersInFunction } from './shared-configuration';

function getHeadersFromEnv(signalIdentifier: string) {
function getStaticHeadersFromEnv(
signalIdentifier: string
): Record<string, string> | undefined {
const signalSpecificRawHeaders =
process.env[`OTEL_EXPORTER_OTLP_${signalIdentifier}_HEADERS`]?.trim();
const nonSignalSpecificRawHeaders =
Expand Down Expand Up @@ -126,6 +129,8 @@ export function getHttpConfigurationFromEnvironment(
url:
getSpecificUrlFromEnv(signalIdentifier) ??
getNonSpecificUrlFromEnv(signalResourcePath),
headers: getHeadersFromEnv(signalIdentifier),
headers: wrapStaticHeadersInFunction(
getStaticHeadersFromEnv(signalIdentifier)
),
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ export function validateTimeoutMillis(timeoutMillis: number) {
);
}

export function wrapStaticHeadersInFunction(
headers: Record<string, string> | undefined
): (() => Record<string, string>) | undefined {
if (headers == null) {
return undefined;
}

return () => headers;
}

/**
* @param userProvidedConfiguration Configuration options provided by the user in code.
* @param fallbackConfiguration Fallback to use when the {@link userProvidedConfiguration} does not specify an option.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function createOtlpSendBeaconExportDelegate<Internal, Response>(
createRetryingTransport({
transport: createSendBeaconTransport({
url: options.url,
blobType: options.headers['Content-Type'],
blobType: options.headers()['Content-Type'],
}),
})
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export type sendWithHttp = (

export interface HttpRequestParameters {
url: string;
headers: Record<string, string>;
headers: () => Record<string, string>;
compression: 'gzip' | 'none';
agentOptions: http.AgentOptions | https.AgentOptions;
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export function sendWithHttp(
path: parsedUrl.pathname,
method: 'POST',
headers: {
...params.headers,
...params.headers(),
},
agent: agent,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {

export interface XhrRequestParameters {
url: string;
headers: Record<string, string>;
headers: () => Record<string, string>;
}

class XhrTransport implements IExporterTransport {
Expand All @@ -35,7 +35,8 @@ class XhrTransport implements IExporterTransport {
const xhr = new XMLHttpRequest();
xhr.timeout = timeoutMillis;
xhr.open('POST', this._parameters.url);
Object.entries(this._parameters.headers).forEach(([k, v]) => {
const headers = this._parameters.headers();
Object.entries(headers).forEach(([k, v]) => {
xhr.setRequestHeader(k, v);
});

Expand Down Expand Up @@ -80,9 +81,7 @@ class XhrTransport implements IExporterTransport {
});
};

xhr.send(
new Blob([data], { type: this._parameters.headers['Content-Type'] })
);
xhr.send(new Blob([data], { type: headers['Content-Type'] }));
});
}

Expand Down
28 changes: 15 additions & 13 deletions experimental/packages/otlp-exporter-base/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,19 @@ import { diag } from '@opentelemetry/api';
* @param partialHeaders
*/
export function validateAndNormalizeHeaders(
partialHeaders: Partial<Record<string, unknown>> = {}
): Record<string, string> {
const headers: Record<string, string> = {};
Object.entries(partialHeaders).forEach(([key, value]) => {
if (typeof value !== 'undefined') {
headers[key] = String(value);
} else {
diag.warn(
`Header "${key}" has invalid value (${value}) and will be ignored`
);
}
});
return headers;
partialHeaders: (() => Record<string, string>) | undefined
): () => Record<string, string> {
return () => {
const headers: Record<string, string> = {};
Object.entries(partialHeaders?.() ?? {}).forEach(([key, value]) => {
if (typeof value !== 'undefined') {
headers[key] = String(value);
} else {
diag.warn(
`Header "${key}" has invalid value (${value}) and will be ignored`
);
}
});
return headers;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ import { ensureHeadersContain } from '../testHelper';

const testTransportParameters = {
url: 'http://example.test',
headers: {
headers: () => ({
foo: 'foo-value',
bar: 'bar-value',
'Content-Type': 'application/json',
},
}),
};

const requestTimeout = 1000;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,22 @@ describe('mergeOtlpHttpConfigurationWithDefaults', function () {
timeoutMillis: 1,
compression: 'none',
concurrencyLimit: 2,
headers: { 'User-Agent': 'default-user-agent' },
headers: () => ({ 'User-Agent': 'default-user-agent' }),
agentOptions: { keepAlive: true },
};

describe('headers', function () {
it('merges headers instead of overriding', function () {
const config = mergeOtlpHttpConfigurationWithDefaults(
{
headers: { foo: 'user' },
headers: () => ({ foo: 'user' }),
},
{
headers: { foo: 'fallback', bar: 'fallback' },
headers: () => ({ foo: 'fallback', bar: 'fallback' }),
},
testDefaults
);
assert.deepStrictEqual(config.headers, {
assert.deepStrictEqual(config.headers(), {
'User-Agent': 'default-user-agent',
foo: 'user',
bar: 'fallback',
Expand All @@ -52,14 +52,14 @@ describe('mergeOtlpHttpConfigurationWithDefaults', function () {
it('does not override default (required) header', function () {
const config = mergeOtlpHttpConfigurationWithDefaults(
{
headers: { 'User-Agent': 'custom' },
headers: () => ({ 'User-Agent': 'custom' }),
},
{
headers: { 'User-Agent': 'custom-fallback' },
headers: () => ({ 'User-Agent': 'custom-fallback' }),
},
testDefaults
);
assert.deepStrictEqual(config.headers, {
assert.deepStrictEqual(config.headers(), {
'User-Agent': 'default-user-agent',
});
});
Expand All @@ -69,12 +69,16 @@ describe('mergeOtlpHttpConfigurationWithDefaults', function () {
{
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore simulating plain JavaScript usage, ignoring types
headers: { foo: 'foo-user-provided', bar: undefined, baz: null },
headers: () => ({
foo: 'foo-user-provided',
bar: undefined,
baz: null,
}),
},
{},
testDefaults
);
assert.deepStrictEqual(config.headers, {
assert.deepStrictEqual(config.headers(), {
foo: 'foo-user-provided',
baz: 'null',
'User-Agent': 'default-user-agent',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ describe('parseHeaders', function () {
foo2: 'bar',
foo3: 1,
};
const result = validateAndNormalizeHeaders(headers);
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore simulating plain JS usage
const result = validateAndNormalizeHeaders(() => headers)();
assert.deepStrictEqual(result, {
foo2: 'bar',
foo3: '1',
Expand All @@ -44,7 +46,7 @@ describe('parseHeaders', function () {
});

it('should parse undefined', function () {
const result = validateAndNormalizeHeaders(undefined);
const result = validateAndNormalizeHeaders(undefined)();
assert.deepStrictEqual(result, {});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('getHttpConfigurationFromEnvironment', function () {
'METRICS',
'v1/metrics'
);
assert.deepEqual(config.headers, {
assert.deepEqual(config.headers?.(), {
key1: 'metrics',
key2: 'value2',
});
Expand All @@ -62,7 +62,7 @@ describe('getHttpConfigurationFromEnvironment', function () {
'METRICS',
'v1/metrics'
);
assert.deepEqual(config.headers, {
assert.deepEqual(config.headers?.(), {
key1: 'value1',
key2: 'value2',
});
Expand All @@ -76,7 +76,7 @@ describe('getHttpConfigurationFromEnvironment', function () {
'METRICS',
'v1/metrics'
);
assert.deepEqual(config.headers, {
assert.deepEqual(config.headers?.(), {
key1: 'value1',
key2: 'value2',
});
Expand Down
Loading

0 comments on commit dd5b5fb

Please sign in to comment.