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.

Sign up for GitHub

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

feat(detector-aws): add DetectorSync implementation for EC2 detector #2332

Closed
wants to merge 4 commits into from
Closed

feat(detector-aws): add DetectorSync implementation for EC2 detector #2332

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
12cdffc
refactor(detector-aws): change implementation to DetectorSync interfa…
david-luna Jul 12, 2024
6b88807
refactor(detector-aws): lint fix
david-luna Jul 12, 2024
75e5374
Merge branch 'main' into dluna/aws-ec2-detector-sync
david-luna Aug 1, 2024
eac609b
feat(detector-aws): add EC2 detector implementing DetectorSync interface
david-luna Aug 1, 2024
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
52 changes: 30 additions & 22 deletions detectors/node/opentelemetry-resource-detector-aws/src/detectors/AwsEc2Detector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
*/

import {
Detector,
DetectorSync,
IResource,
Resource,
ResourceAttributes,
ResourceDetectionConfig,
} from '@opentelemetry/resources';
import {
Expand All @@ -38,7 +40,7 @@ import * as http from 'http';
* and return a {@link Resource} populated with metadata about the EC2
* instance. Returns an empty Resource if detection fails.
*/
class AwsEc2Detector implements Detector {
class AwsEc2Detector implements DetectorSync {
/**
* See https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-identity-documents.html
* for documentation about the AWS instance identity document
Expand All @@ -53,31 +55,37 @@ class AwsEc2Detector implements Detector {
readonly AWS_METADATA_TOKEN_HEADER = 'X-aws-ec2-metadata-token';
readonly MILLISECOND_TIME_OUT = 5000;

detect(_config?: ResourceDetectionConfig): IResource {
return new Resource({}, this._getAttributes());
}

/**
* Attempts to connect and obtain an AWS instance Identity document. If the
* connection is successful it returns a promise containing a {@link Resource}
* populated with instance metadata. Returns a promise containing an
* empty {@link Resource} if the connection or parsing of the identity
* connection is successful it returns a promise containing a {@link ResourceAttributes}
* object with instance metadata. Returns a promise containing an
* empty {@link ResourceAttributes} if the connection or parsing of the identity
* document fails.
*
* @param config (unused) The resource detection config
*/
async detect(_config?: ResourceDetectionConfig): Promise<Resource> {
const token = await this._fetchToken();
const { accountId, instanceId, instanceType, region, availabilityZone } =
await this._fetchIdentity(token);
const hostname = await this._fetchHost(token);
async _getAttributes(): Promise<ResourceAttributes> {
try {
const token = await this._fetchToken();
const { accountId, instanceId, instanceType, region, availabilityZone } =
await this._fetchIdentity(token);
const hostname = await this._fetchHost(token);

return new Resource({
[SEMRESATTRS_CLOUD_PROVIDER]: CLOUDPROVIDERVALUES_AWS,
[SEMRESATTRS_CLOUD_PLATFORM]: CLOUDPLATFORMVALUES_AWS_EC2,
[SEMRESATTRS_CLOUD_ACCOUNT_ID]: accountId,
[SEMRESATTRS_CLOUD_REGION]: region,
[SEMRESATTRS_CLOUD_AVAILABILITY_ZONE]: availabilityZone,
[SEMRESATTRS_HOST_ID]: instanceId,
[SEMRESATTRS_HOST_TYPE]: instanceType,
[SEMRESATTRS_HOST_NAME]: hostname,
});
return {
[SEMRESATTRS_CLOUD_PROVIDER]: CLOUDPROVIDERVALUES_AWS,
[SEMRESATTRS_CLOUD_PLATFORM]: CLOUDPLATFORMVALUES_AWS_EC2,
[SEMRESATTRS_CLOUD_ACCOUNT_ID]: accountId,
[SEMRESATTRS_CLOUD_REGION]: region,
[SEMRESATTRS_CLOUD_AVAILABILITY_ZONE]: availabilityZone,
[SEMRESATTRS_HOST_ID]: instanceId,
[SEMRESATTRS_HOST_TYPE]: instanceType,
[SEMRESATTRS_HOST_NAME]: hostname,
};
} catch {
return {};
}
}

private async _fetchToken(): Promise<string> {
Expand Down
51 changes: 20 additions & 31 deletions detectors/node/opentelemetry-resource-detector-aws/test/detectors/AwsEc2Detector.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import * as nock from 'nock';
import * as assert from 'assert';
import { Resource } from '@opentelemetry/resources';

import { awsEc2Detector } from '../../src';
import {
assertCloudResource,
Expand Down Expand Up @@ -64,7 +64,8 @@ describe('awsEc2Detector', () => {
.matchHeader(AWS_METADATA_TOKEN_HEADER, mockedTokenResponse)
.reply(200, () => mockedHostResponse);

const resource: Resource = await awsEc2Detector.detect();
const resource = awsEc2Detector.detect();
await resource.waitForAsyncAttributes?.();

scope.done();

Expand All @@ -85,8 +86,7 @@ describe('awsEc2Detector', () => {
});

describe('with unsuccessful request', () => {
it('should throw when receiving error response code', async () => {
const expectedError = new Error('Failed to load page, status code: 404');
it('should return empty resource when receiving error response code', async () => {
const scope = nock(AWS_HOST)
.persist()
.put(AWS_TOKEN_PATH)
Expand All @@ -99,19 +99,16 @@ describe('awsEc2Detector', () => {
.matchHeader(AWS_METADATA_TOKEN_HEADER, mockedTokenResponse)
.reply(404, () => new Error());

try {
await awsEc2Detector.detect();
assert.ok(false, 'Expected to throw');
} catch (err) {
assert.deepStrictEqual(err, expectedError);
}
const resource = awsEc2Detector.detect();
await resource.waitForAsyncAttributes?.();

assert.deepStrictEqual(resource.attributes, {});

scope.done();
});

it('should throw when timed out', function (done) {
it('should return empty resource when timed out', async function () {
this.timeout(6000);
const expectedError = new Error('EC2 metadata api request timed out.');
const scope = nock(AWS_HOST)
.put(AWS_TOKEN_PATH)
.matchHeader(AWS_METADATA_TTL_HEADER, '60')
Expand All @@ -124,21 +121,15 @@ describe('awsEc2Detector', () => {
.delayConnection(5000)
.reply(200, () => mockedHostResponse);

awsEc2Detector
.detect()
.then(() => {
assert.ok(false, 'Expected to throw');
})
.catch(err => {
assert.deepStrictEqual(err, expectedError);
})
.finally(() => {
scope.done();
done();
});
const resource = awsEc2Detector.detect();
await resource.waitForAsyncAttributes?.();

assert.deepStrictEqual(resource.attributes, {});

scope.done();
});

it('should throw when replied with an Error', async () => {
it('should return empty resource when replied with an Error', async () => {
const expectedError = new Error('NOT FOUND');
const scope = nock(AWS_HOST)
.put(AWS_TOKEN_PATH)
Expand All @@ -148,12 +139,10 @@ describe('awsEc2Detector', () => {
.matchHeader(AWS_METADATA_TOKEN_HEADER, mockedTokenResponse)
.replyWithError(expectedError.message);

try {
await awsEc2Detector.detect();
assert.ok(false, 'Expected to throw');
} catch (err) {
assert.deepStrictEqual(err, expectedError);
}
const resource = awsEc2Detector.detect();
await resource.waitForAsyncAttributes?.();

assert.deepStrictEqual(resource.attributes, {});

scope.done();
});
Expand Down
6 changes: 5 additions & 1 deletion metapackages/auto-instrumentations-node/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,11 @@ function getDisabledInstrumentationsFromEnv() {
export function getResourceDetectorsFromEnv(): Array<Detector | DetectorSync> {
const resourceDetectors = new Map<
string,
Detector | DetectorSync | Detector[] | DetectorSync[]
| Detector
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: this is necessary to allow this single refactor instead of doing all AWS detector at once.

| DetectorSync
| Detector[]
| DetectorSync[]
| Array<Detector | DetectorSync>
>([
[RESOURCE_DETECTOR_CONTAINER, containerDetector],
[RESOURCE_DETECTOR_ENVIRONMENT, envDetectorSync],
Expand Down