Skip to content

Commit

Permalink
GDI spec 1.6.0 compliance (#968)
Browse files Browse the repository at this point in the history
* gdi compliance update

* update readme
  • Loading branch information
seemk authored Nov 12, 2024
1 parent a3df29a commit da88406
Show file tree
Hide file tree
Showing 12 changed files with 57 additions and 63 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
<a href="https://github.com/signalfx/splunk-otel-js/releases">
<img alt="GitHub release (latest by date)" src="https://img.shields.io/github/v/release/signalfx/splunk-otel-js?include_prereleases&style=for-the-badge">
</a>
<a href="https://github.com/signalfx/gdi-specification/releases/tag/v1.2.0">
<img alt="Splunk GDI specification" src="https://img.shields.io/badge/GDI-1.2.0-blueviolet?style=for-the-badge">
<a href="https://github.com/signalfx/gdi-specification/releases/tag/v1.6.0">
<img alt="Splunk GDI specification" src="https://img.shields.io/badge/GDI-1.6.0-blueviolet?style=for-the-badge">
</a>
<img alt="npm" src="https://img.shields.io/npm/v/@splunk/otel?style=for-the-badge">
<img alt="node-current" src="https://img.shields.io/node/v/@splunk/otel?style=for-the-badge">
Expand Down
7 changes: 2 additions & 5 deletions src/detectors/DistroDetector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,11 @@ import {
} from '@opentelemetry/resources';
import { VERSION } from '../version';

/**
* DistroDetector will be used to detect `splunk.distro.version` and other
* distro-related resource information.
*/
class DistroDetector {
detect(_config?: ResourceDetectionConfig): Resource {
const distroAttributes: ResourceAttributes = {
'splunk.distro.version': VERSION,
'telemetry.distro.name': 'splunk-nodejs',
'telemetry.distro.version': VERSION,
};
return new Resource(distroAttributes);
}
Expand Down
9 changes: 0 additions & 9 deletions src/logging/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import * as util from 'util';
import * as logsAPI from '@opentelemetry/api-logs';
import { OTLPLogExporter } from '@opentelemetry/exporter-logs-otlp-http';
import { Resource } from '@opentelemetry/resources';
import { diag } from '@opentelemetry/api';
import { ATTR_SERVICE_NAME } from '@opentelemetry/semantic-conventions';
import {
LoggerProvider,
Expand Down Expand Up @@ -72,14 +71,6 @@ export function _setDefaultOptions(
getNonEmptyEnvVar('OTEL_SERVICE_NAME') ||
envResource.attributes[ATTR_SERVICE_NAME];

if (!serviceName) {
diag.warn(
'service.name attribute for logging is not set, your service is unnamed and will be difficult to identify. ' +
'Set your service name using the OTEL_RESOURCE_ATTRIBUTES environment variable. ' +
'E.g. OTEL_RESOURCE_ATTRIBUTES="service.name=<YOUR_SERVICE_NAME_HERE>"'
);
}

const resourceFactory = options.resourceFactory || ((r: Resource) => r);
const resource = resourceFactory(envResource).merge(
new Resource({
Expand Down
12 changes: 7 additions & 5 deletions src/metrics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,12 @@ export function createOtlpExporter(options: MetricsOptions) {
);
}

if (options.endpoint === undefined) {
const envEndpoint = getEnvValueByPrecedence([
'OTEL_EXPORTER_OTLP_METRICS_ENDPOINT',
'OTEL_EXPORTER_OTLP_ENDPOINT',
]);

if (endpoint === undefined && envEndpoint === undefined) {
endpoint = `https://ingest.${options.realm}.signalfx.com/v2/datapoint/otlp`;
protocol = 'http/protobuf';
} else {
Expand Down Expand Up @@ -366,9 +371,6 @@ export function _setDefaultOptions(
const accessToken =
options.accessToken || getNonEmptyEnvVar('SPLUNK_ACCESS_TOKEN') || '';

const endpoint =
options.endpoint || getNonEmptyEnvVar('SPLUNK_METRICS_ENDPOINT');

const realm = options.realm || getNonEmptyEnvVar('SPLUNK_REALM') || '';

if (realm) {
Expand Down Expand Up @@ -408,7 +410,7 @@ export function _setDefaultOptions(
accessToken,
realm,
resource,
endpoint,
endpoint: options.endpoint,
views: options.views,
metricReaderFactory:
options.metricReaderFactory ?? defaultMetricReaderFactory,
Expand Down
17 changes: 17 additions & 0 deletions src/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ import {
MeterOptions,
createNoopMeter,
} from '@opentelemetry/api';
import { ATTR_SERVICE_NAME } from '@opentelemetry/semantic-conventions';
import { StartLoggingOptions, startLogging } from './logging';
import { Resource } from '@opentelemetry/resources';
import { getDetectedResource } from './resource';

export interface Options {
accessToken: string;
Expand Down Expand Up @@ -94,6 +96,21 @@ export const start = (options: Partial<Options> = {}) => {
diag.setLogger(new DiagConsoleLogger(), logLevel);
}

const envResource = getDetectedResource();

const serviceName =
options.serviceName ||
getNonEmptyEnvVar('OTEL_SERVICE_NAME') ||
envResource.attributes[ATTR_SERVICE_NAME];

if (!serviceName) {
diag.warn(
'service.name attribute is not set, your service is unnamed and will be difficult to identify. ' +
'Set your service name using the OTEL_RESOURCE_ATTRIBUTES environment variable. ' +
'E.g. OTEL_RESOURCE_ATTRIBUTES="service.name=<YOUR_SERVICE_NAME_HERE>"'
);
}

const { tracingOptions, loggingOptions, profilingOptions, metricsOptions } =
parseOptionsAndConfigureInstrumentations(options);

Expand Down
8 changes: 0 additions & 8 deletions src/tracing/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,6 @@ export function _setDefaultOptions(
getNonEmptyEnvVar('OTEL_SERVICE_NAME') ||
resource.attributes[ATTR_SERVICE_NAME];

if (!serviceName) {
diag.warn(
'service.name attribute is not set, your service is unnamed and will be difficult to identify. ' +
'Set your service name using the OTEL_RESOURCE_ATTRIBUTES environment variable. ' +
'E.g. OTEL_RESOURCE_ATTRIBUTES="service.name=<YOUR_SERVICE_NAME_HERE>"'
);
}

resource = resource.merge(
new Resource({
[ATTR_SERVICE_NAME]: serviceName || defaultServiceName(),
Expand Down
2 changes: 1 addition & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export type EnvVarKey =
| 'OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE'
| 'OTEL_EXPORTER_OTLP_CLIENT_KEY'
| 'OTEL_EXPORTER_OTLP_ENDPOINT'
| 'OTEL_EXPORTER_OTLP_METRICS_ENDPOINT'
| 'OTEL_EXPORTER_OTLP_METRICS_PROTOCOL'
| 'OTEL_EXPORTER_OTLP_PROTOCOL'
| 'OTEL_EXPORTER_OTLP_TRACES_ENDPOINT'
Expand All @@ -47,7 +48,6 @@ export type EnvVarKey =
| 'SPLUNK_GRAPHQL_RESOLVE_SPANS_ENABLED'
| 'SPLUNK_INSTRUMENTATION_METRICS_ENABLED'
| 'SPLUNK_METRICS_ENABLED'
| 'SPLUNK_METRICS_ENDPOINT'
| 'SPLUNK_NEXTJS_FIX_ENABLED'
| 'SPLUNK_PROFILER_CALL_STACK_INTERVAL'
| 'SPLUNK_PROFILER_ENABLED'
Expand Down
9 changes: 0 additions & 9 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,15 +427,6 @@ export function listEnvVars() {
type: 'boolean',
category: 'metrics',
},
{
name: 'SPLUNK_METRICS_ENDPOINT',
property: 'metrics.endpoint',
description:
'The metrics endpoint. Takes precedence over OTEL_EXPORTER_OTLP_METRICS_ENDPOINT. When SPLUNK_REALM is used, the default value is https://ingest.<realm>.signalfx.com/v2/datapoint/otlp.',
default: '',
type: 'string',
category: 'metrics',
},
{
name: 'SPLUNK_PROFILER_ENABLED',
property: 'profilingEnabled',
Expand Down
2 changes: 1 addition & 1 deletion test/metrics.options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ describe('metrics options', () => {
it('warns when realm and endpoint are both set', () => {
process.env.SPLUNK_REALM = 'us0';
process.env.SPLUNK_ACCESS_TOKEN = 'abc';
process.env.SPLUNK_METRICS_ENDPOINT = 'http://localhost:4320';
process.env.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://localhost:4320';

const options = _setDefaultOptions();
const [reader] = options.metricReaderFactory(options);
Expand Down
35 changes: 12 additions & 23 deletions test/options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,6 @@ const assertContainerId = (containerIdAttr) => {
`${containerIdAttr} is not an hex string`
);
};
/*
service.name attribute is not set, your service is unnamed and will be difficult to identify.
Set your service name using the OTEL_RESOURCE_ATTRIBUTES environment variable.
E.g. OTEL_RESOURCE_ATTRIBUTES="service.name=<YOUR_SERVICE_NAME_HERE>"
*/
const MATCH_SERVICE_NAME_WARNING = /service\.name.*not.*set/i;

describe('options', () => {
let logger;
Expand Down Expand Up @@ -111,9 +105,12 @@ describe('options', () => {
it('has expected defaults', () => {
const options = _setDefaultOptions();

assertVersion(
options.tracerConfig.resource?.attributes['splunk.distro.version']
);
const resAttrs =
options.tracerConfig.resource?.attributes || Resource.empty();

assertVersion(resAttrs['telemetry.distro.version']);

assert.strictEqual(resAttrs['telemetry.distro.name'], 'splunk-nodejs');

const expectedAttributes = new Set([
ATTR_HOST_ARCH,
Expand All @@ -124,27 +121,22 @@ describe('options', () => {
ATTR_PROCESS_PID,
ATTR_PROCESS_RUNTIME_NAME,
ATTR_PROCESS_RUNTIME_VERSION,
'splunk.distro.version',
'telemetry.distro.version',
'telemetry.distro.name',
]);

expectedAttributes.forEach((processAttribute) => {
assert(
options.tracerConfig.resource?.attributes[processAttribute],
`${processAttribute} missing`
);
assert(resAttrs[processAttribute], `${processAttribute} missing`);
});

assert.deepStrictEqual(
options.tracerConfig.resource?.attributes[ATTR_SERVICE_NAME],
'@splunk/otel'
);
assert.deepStrictEqual(resAttrs[ATTR_SERVICE_NAME], '@splunk/otel');

// Container ID is checked in a different test,
// this avoids collisions with stubbing fs methods.
delete options.tracerConfig.resource.attributes[ATTR_CONTAINER_ID];
delete resAttrs[ATTR_CONTAINER_ID];

// resource attributes for process, host and os are different at each run, iterate through them, make sure they exist and then delete
Object.keys(options.tracerConfig.resource.attributes)
Object.keys(resAttrs)
.filter((attribute) => {
return expectedAttributes.has(attribute);
})
Expand Down Expand Up @@ -184,9 +176,6 @@ describe('options', () => {
const [exporter] = exporters;

assert(exporter instanceof OTLPTraceExporter);

const logMsg = logger.warn.mock.calls[0].arguments[0];
assert(MATCH_SERVICE_NAME_WARNING.test(logMsg));
});

it('reads the container when setting default options', async () => {
Expand Down
2 changes: 2 additions & 0 deletions test/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ const args = [
'--require',
'ts-node/register/transpile-only',
'--test',
'--test-reporter',
'spec',
...testFiles,
];

Expand Down
13 changes: 13 additions & 0 deletions test/start.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,18 +269,21 @@ describe('start', () => {
describe('diagnostic logging', () => {
let logSpy;
let infoSpy;
let warnSpy;
let debugSpy;

beforeEach(() => {
utils.cleanEnvironment();
logSpy = mock.method(console, 'log');
infoSpy = mock.method(console, 'info');
warnSpy = mock.method(console, 'warn');
debugSpy = mock.method(console, 'debug');
});

afterEach(() => {
logSpy.mock.resetCalls();
infoSpy.mock.resetCalls();
warnSpy.mock.resetCalls();
debugSpy.mock.resetCalls();
});

Expand Down Expand Up @@ -315,5 +318,15 @@ describe('start', () => {
diag.debug('42');
assert(debugSpy.mock.callCount() === 0);
});

it('logs a warning when service name is not set', () => {
start({ logLevel: 'info' });
utils.calledWithExactly(
warnSpy,
'service.name attribute is not set, your service is unnamed and will be difficult to identify. ' +
'Set your service name using the OTEL_RESOURCE_ATTRIBUTES environment variable. ' +
'E.g. OTEL_RESOURCE_ATTRIBUTES="service.name=<YOUR_SERVICE_NAME_HERE>"'
);
});
});
});

0 comments on commit da88406

Please sign in to comment.