-
Notifications
You must be signed in to change notification settings - Fork 954
feat(metrics): add advisory attributes parameter to metric instruments #5783
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
bad0c9f
09c03bd
82948e1
d7c17af
957e0bf
b5eca96
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| /* | ||
| * This example demonstrates the new advisory attributes parameter for metrics instruments. | ||
| * The attributes parameter acts as an allow-list for the instrument, filtering out | ||
| * all attribute keys that are not in the specified list. | ||
| */ | ||
|
|
||
| const { MeterProvider } = require('@opentelemetry/sdk-metrics'); | ||
| const { ConsoleMetricExporter } = require('@opentelemetry/sdk-metrics'); | ||
| const { PeriodicExportingMetricReader } = require('@opentelemetry/sdk-metrics'); | ||
|
|
||
| // Create a meter provider with a console exporter | ||
| const meterProvider = new MeterProvider({ | ||
| readers: [ | ||
| new PeriodicExportingMetricReader({ | ||
| exporter: new ConsoleMetricExporter(), | ||
| exportIntervalMillis: 1000, | ||
| }), | ||
| ], | ||
| }); | ||
|
|
||
| const meter = meterProvider.getMeter('advisory-attributes-example'); | ||
|
|
||
| // Create a counter with advisory attributes - only 'service' and 'version' keys will be kept | ||
| const requestCounter = meter.createCounter('http_requests_total', { | ||
| description: 'Total number of HTTP requests', | ||
| advice: { | ||
| attributes: ['service', 'version'], // @experimental: Only these keys will be allowed | ||
| }, | ||
| }); | ||
|
|
||
| // Create a histogram without advisory attributes - all keys will be kept | ||
| const responseTimeHistogram = meter.createHistogram('http_response_time', { | ||
| description: 'HTTP response time in milliseconds', | ||
| }); | ||
|
|
||
| // Record some measurements | ||
| console.log('Recording metrics with advisory attributes filtering...\n'); | ||
|
|
||
| // This will only keep 'service' and 'version' attributes, filtering out 'method' and 'endpoint' | ||
| requestCounter.add(1, { | ||
| service: 'api-gateway', | ||
| version: '1.2.3', | ||
| method: 'GET', // This will be filtered out | ||
| endpoint: '/users', // This will be filtered out | ||
| }); | ||
|
|
||
| requestCounter.add(1, { | ||
| service: 'user-service', | ||
| version: '2.1.0', | ||
| method: 'POST', // This will be filtered out | ||
| endpoint: '/auth', // This will be filtered out | ||
| region: 'us-west-2', // This will be filtered out | ||
| }); | ||
|
|
||
| // This will keep all attributes since no advisory attributes are specified | ||
| responseTimeHistogram.record(150, { | ||
| service: 'api-gateway', | ||
| method: 'GET', | ||
| endpoint: '/users', | ||
| status_code: '200', | ||
| }); | ||
|
|
||
| responseTimeHistogram.record(89, { | ||
| service: 'user-service', | ||
| method: 'POST', | ||
| endpoint: '/auth', | ||
| status_code: '201', | ||
| region: 'us-west-2', | ||
| }); | ||
|
|
||
| console.log('Check the console output above to see the filtered attributes!'); | ||
| console.log('The http_requests_total metric should only have service and version attributes.'); | ||
| console.log('The http_response_time metric should have all attributes.'); | ||
|
|
||
| // Keep the process running for a bit to see the output | ||
| setTimeout(() => { | ||
| console.log('\nShutting down...'); | ||
| meterProvider.shutdown(); | ||
| }, 2000); | ||
pratstick marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ import { InstrumentDescriptor } from '../InstrumentDescriptor'; | |
| import { InstrumentSelector } from './InstrumentSelector'; | ||
| import { MeterSelector } from './MeterSelector'; | ||
| import { View } from './View'; | ||
| import { createAllowListAttributesProcessor } from './AttributesProcessor'; | ||
|
|
||
| export class ViewRegistry { | ||
| private _registeredViews: View[] = []; | ||
|
|
@@ -38,9 +39,32 @@ export class ViewRegistry { | |
| ); | ||
| }); | ||
|
|
||
| // If no registered views match, create a default view with instrument's advisory attributes | ||
| if (views.length === 0) { | ||
| const defaultView = this._createDefaultView(instrument); | ||
| return [defaultView]; | ||
| } | ||
|
|
||
| return views; | ||
| } | ||
|
|
||
| private _createDefaultView(instrument: InstrumentDescriptor): View { | ||
| // Create default view with instrument's advisory attributes as an allow-list | ||
| const viewOptions: any = { | ||
| instrumentName: instrument.name, | ||
| instrumentType: instrument.type, | ||
| instrumentUnit: instrument.unit, | ||
| }; | ||
|
|
||
| // If instrument has advisory attributes, use them as an allow-list | ||
| if (instrument.advice.attributes && instrument.advice.attributes.length > 0) { | ||
| viewOptions.attributesProcessors = [createAllowListAttributesProcessor(instrument.advice.attributes)]; | ||
| } | ||
|
|
||
| // Create a view that matches this specific instrument | ||
| return new View(viewOptions); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @pichlermarc Thank you
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
|
||
| 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', | ||
| }); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.