Skip to content
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

fix(detector-alibaba): suppres internal tracing #2429

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"@opentelemetry/api": "^1.0.0"
},
"dependencies": {
"@opentelemetry/resources": "^1.0.0",
"@opentelemetry/resources": "^1.10.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note for reviewer: DetectorSync interface appeared in v1.10.0. this is a leftover from #2328

"@opentelemetry/semantic-conventions": "^1.27.0"
},
"homepage": "https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/detectors/node/opentelemetry-resource-detector-alibaba-cloud#readme"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
* limitations under the License.
*/

import { context } from '@opentelemetry/api';
import { suppressTracing } from '@opentelemetry/core';
import {
DetectorSync,
IResource,
Expand Down Expand Up @@ -61,7 +63,10 @@ class AlibabaCloudEcsDetector implements DetectorSync {
* @param config (unused) The resource detection config
*/
detect(_config?: ResourceDetectionConfig): IResource {
return new Resource({}, this._getAttributes());
const attributes = context.with(suppressTracing(context.active()), () =>
this._getAttributes()
);
return new Resource({}, attributes);
}

/** Gets identity and host info and returns them as attribs. Empty object if fails */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* 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 { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import {
InMemorySpanExporter,
SimpleSpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import { NodeSDK } from '@opentelemetry/sdk-node';
import { IResource } from '@opentelemetry/resources';

describe('[Integration] AlibabaCloudEcsDetector', () => {
it('should not start spans for detector requests', async () => {
const memoryExporter = new InMemorySpanExporter();
const sdk = new NodeSDK({
instrumentations: [new HttpInstrumentation()],
spanProcessors: [new SimpleSpanProcessor(memoryExporter)],
});

sdk.start();

// NOTE: detectors implementing the `DetectorSync` interface and starting
// HTTP requests within the `detect` method will produce Noop Spans since
// the SDK resolves the trace provider after resource detectors are triggered.
// Ref: https://github.com/open-telemetry/opentelemetry-js/blob/38f6689480d28dcbdafcb7b5ba4b14025328ffda/experimental/packages/opentelemetry-sdk-node/src/sdk.ts#L210-L240
//
// So having the detector in the config would result in no spans for Alibaba requests
// being exported which is what we want. Although we may think we're safe of sending
// internal tracing any change that delays these request will result in internal
// tracing being exported. We do the detection outside the SDK constructor to have such
// scenario.
const {
alibabaCloudEcsDetector,
} = require('../../build/src/detectors/AlibabaCloudEcsDetector');
const resource = alibabaCloudEcsDetector.detect() as IResource;
await resource.waitForAsyncAttributes?.();

// Wait for the next loop to let the span close properly
await new Promise(r => setTimeout(r, 0));
const spans = memoryExporter.getFinishedSpans();

assert.equal(spans.length, 0, 'no spans exported for GcpDetector');

await sdk.shutdown();
});
});