Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2

### :rocket: Features

* feat(metrics): add advisory attributes parameter to metric instruments [#4365](https://github.com/open-telemetry/opentelemetry-js/issues/4365)
* Added experimental `attributes` parameter to `MetricAdvice` for filtering measurement attributes
* Only applies when no Views are configured for the instrument
* Views take precedence over advisory attributes when present

### :bug: Bug Fixes

* fix(sdk-metrics): Remove invalid default value for `startTime` param to ExponentialHistogramAccumulation. This only impacted the closurescript compiler. [#5763](https://github.com/open-telemetry/opentelemetry-js/pull/5763) @trentm
Expand Down
4 changes: 4 additions & 0 deletions api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ All notable changes to this project will be documented in this file.

### :rocket: (Enhancement)

* feat(metrics): add advisory attributes parameter to metric instruments [#4365](https://github.com/open-telemetry/opentelemetry-js/issues/4365)
* Added experimental `attributes` parameter to `MetricAdvice` interface
* Allows specifying an allow-list of attribute keys for metric instruments

* feat(api): improve isValidSpanId, isValidTraceId performance [#5714](https://github.com/open-telemetry/opentelemetry-js/pull/5714) @seemk
* feat(diag): change types in `DiagComponentLogger` from `any` to `unknown`[#5478](https://github.com/open-telemetry/opentelemetry-js/pull/5478) @loganrosen

Expand Down
12 changes: 12 additions & 0 deletions api/src/metrics/Metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ export interface MetricAdvice {
* aggregated with a HistogramAggregator.
*/
explicitBucketBoundaries?: number[];

/**
* @experimental
* An allow-list of attribute keys for this instrument. Only these keys will be kept
* on measurements recorded by this instrument. If not provided, all attributes are kept.
*
* @example <caption>only keep 'service' and 'version' attributes</caption>
* attributes: ['service', 'version']
* @example <caption>keep all attributes (default behavior)</caption>
* attributes: undefined
*/
attributes?: string[];
}

/**
Expand Down
23 changes: 23 additions & 0 deletions packages/sdk-metrics/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,29 @@ const meterProvider = new MeterProvider({
})
```

## Advisory Attributes (Experimental)

The Metrics API supports an optional `attributes` advisory parameter in the `advice` configuration when creating instruments. This parameter acts as an allow-list for attribute keys, filtering out all attributes that are not in the specified list:

```js
const counter = opentelemetry.metrics.getMeter('default').createCounter('filtered-counter', {
description: 'A counter with attribute filtering',
advice: {
attributes: ['service', 'version'], // @experimental: Only these keys will be kept
},
});

// Only 'service' and 'version' attributes will be recorded, others are filtered out
counter.add(1, {
service: 'api-gateway',
version: '1.0.0',
region: 'us-west', // This will be filtered out
method: 'GET' // This will be filtered out
});
```

**Note:** This feature is experimental and may change in future versions. The advisory attributes parameter only applies when no Views are configured for the instrument. If Views are present, they take precedence over the instrument's advisory attributes.

## Example

See [examples/prometheus](https://github.com/open-telemetry/opentelemetry-js/tree/main/experimental/examples/prometheus) for an end-to-end example, including exporting metrics.
Expand Down
37 changes: 35 additions & 2 deletions packages/sdk-metrics/src/view/ViewRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import { InstrumentationScope } from '@opentelemetry/core';
import { InstrumentDescriptor } from '../InstrumentDescriptor';
import { InstrumentSelector } from './InstrumentSelector';
import { MeterSelector } from './MeterSelector';
import { View } from './View';
import { View, ViewOptions } from './View';
import { createAllowListAttributesProcessor } from './AttributesProcessor';

export class ViewRegistry {
private _registeredViews: View[] = [];
Expand All @@ -38,9 +39,41 @@ export class ViewRegistry {
);
});

// Only create a default view if advisory attributes are set and non-empty
if (views.length === 0) {
if (
instrument.advice &&
Array.isArray(instrument.advice.attributes) &&
instrument.advice.attributes.length > 0
) {
return [this._createDefaultView(instrument)];
}
// No matching views and no advisory attributes: return empty array for backward compatibility
return [];
}

return views;
}

private _createDefaultView(instrument: InstrumentDescriptor): View {
const viewOptions: ViewOptions = {
instrumentName: instrument.name,
instrumentType: instrument.type,
instrumentUnit: instrument.unit,
};

if (
instrument.advice.attributes &&
instrument.advice.attributes.length > 0
) {
viewOptions.attributesProcessors = [
createAllowListAttributesProcessor(instrument.advice.attributes),
];
}

return new View(viewOptions);
Copy link
Member

Choose a reason for hiding this comment

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

My guess is that creating a view always is what's eventually failing the tests. You might have to look into which part of the code calls findViews() to investigate what's wrong.

Copy link
Author

Choose a reason for hiding this comment

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

Sure , I will look into all the issues and get back to you

Copy link
Author

Choose a reason for hiding this comment

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

Hi @pichlermarc
I've addressed the aforementioned issues
Could you take a look

Thank you

Copy link
Member

Choose a reason for hiding this comment

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

looks like it's still failing due to the issue I mentioned above, did you force-push over the change that addressed this? 🤔

}

private _matchInstrument(
selector: InstrumentSelector,
instrument: InstrumentDescriptor
Expand All @@ -65,4 +98,4 @@ export class ViewRegistry {
selector.getSchemaUrlFilter().match(meter.schemaUrl))
);
}
}
}
194 changes: 194 additions & 0 deletions packages/sdk-metrics/test/advisory-attributes.integration.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
/*
* 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.
*/

import * as assert from 'assert';
import { MeterProvider } from '../src/MeterProvider';
import { TestMetricReader } from './export/TestMetricReader';
import {
defaultInstrumentationScope,
testResource,
} from './util';

describe('Advisory attributes parameter - Integration test', () => {
let meterProvider: MeterProvider;
let metricReader: TestMetricReader;

beforeEach(() => {
metricReader = new TestMetricReader();
meterProvider = new MeterProvider({
resource: testResource,
readers: [metricReader],
});
});

afterEach(() => {
metricReader.shutdown();
});

it('should filter attributes based on advisory attributes parameter', async () => {
const meter = meterProvider.getMeter(
defaultInstrumentationScope.name,
defaultInstrumentationScope.version,
{
schemaUrl: defaultInstrumentationScope.schemaUrl,
}
);

// Create counter with advisory attributes
const counter = meter.createCounter('test_counter', {
description: 'Test counter with advisory attributes',
advice: {
attributes: ['allowed_key1', 'allowed_key2'], // @experimental
},
});

// Record measurements with various attributes
counter.add(1, {
allowed_key1: 'value1',
allowed_key2: 'value2',
filtered_key: 'filtered_value', // This should be filtered out
});

counter.add(2, {
allowed_key1: 'value3',
another_filtered_key: 'another_filtered_value', // This should be filtered out
});

// Collect metrics
const metrics = await metricReader.collect();

assert.strictEqual(metrics.resourceMetrics.scopeMetrics.length, 1);

const scopeMetrics = metrics.resourceMetrics.scopeMetrics[0];
assert.strictEqual(scopeMetrics.metrics.length, 1);

const metric = scopeMetrics.metrics[0];
assert.strictEqual(metric.descriptor.name, 'test_counter');
assert.strictEqual(metric.dataPoints.length, 2);

// Verify that only allowed attributes are present
const firstDataPoint = metric.dataPoints[0];
assert.deepStrictEqual(firstDataPoint.attributes, {
allowed_key1: 'value1',
allowed_key2: 'value2',
});

const secondDataPoint = metric.dataPoints[1];
assert.deepStrictEqual(secondDataPoint.attributes, {
allowed_key1: 'value3',
});
});

it('should keep all attributes when no advisory attributes are specified', async () => {
const meter = meterProvider.getMeter(
defaultInstrumentationScope.name,
defaultInstrumentationScope.version,
{
schemaUrl: defaultInstrumentationScope.schemaUrl,
}
);

// Create counter without advisory attributes
const counter = meter.createCounter('test_counter_no_filter', {
description: 'Test counter without advisory attributes',
});

// Record measurements with various attributes
counter.add(1, {
key1: 'value1',
key2: 'value2',
key3: 'value3',
});

// Collect metrics
const metrics = await metricReader.collect();

assert.strictEqual(metrics.resourceMetrics.scopeMetrics.length, 1);

const scopeMetrics = metrics.resourceMetrics.scopeMetrics[0];
assert.strictEqual(scopeMetrics.metrics.length, 1);

const metric = scopeMetrics.metrics[0];
assert.strictEqual(metric.descriptor.name, 'test_counter_no_filter');
assert.strictEqual(metric.dataPoints.length, 1);

// Verify that all attributes are present
const dataPoint = metric.dataPoints[0];
assert.deepStrictEqual(dataPoint.attributes, {
key1: 'value1',
key2: 'value2',
key3: 'value3',
});
});

it('should work with different instrument types', async () => {
const meter = meterProvider.getMeter(
defaultInstrumentationScope.name,
defaultInstrumentationScope.version,
{
schemaUrl: defaultInstrumentationScope.schemaUrl,
}
);

// Test with Histogram
const histogram = meter.createHistogram('test_histogram', {
description: 'Test histogram with advisory attributes',
advice: {
attributes: ['service'], // @experimental
},
});

histogram.record(10, {
service: 'api',
endpoint: '/users', // This should be filtered out
});

// Test with UpDownCounter
const upDownCounter = meter.createUpDownCounter('test_updown', {
description: 'Test updown counter with advisory attributes',
advice: {
attributes: ['region'], // @experimental
},
});

upDownCounter.add(5, {
region: 'us-west',
instance: 'i-12345', // This should be filtered out
});

// Collect metrics
const metrics = await metricReader.collect();

assert.strictEqual(metrics.resourceMetrics.scopeMetrics.length, 1);

const scopeMetrics = metrics.resourceMetrics.scopeMetrics[0];
assert.strictEqual(scopeMetrics.metrics.length, 2);

// Verify histogram filtering
const histogramMetric = scopeMetrics.metrics.find((m: any) => m.descriptor.name === 'test_histogram');
assert.ok(histogramMetric);
assert.deepStrictEqual(histogramMetric.dataPoints[0].attributes, {
service: 'api',
});

// Verify updown counter filtering
const upDownMetric = scopeMetrics.metrics.find((m: any) => m.descriptor.name === 'test_updown');
assert.ok(upDownMetric);
assert.deepStrictEqual(upDownMetric.dataPoints[0].attributes, {
region: 'us-west',
});
});
});
Loading
Loading