-
Notifications
You must be signed in to change notification settings - Fork 823
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
Feat: Migrate EC2 Plugin Resource Detector from IMDSv1 to IMDSv2 #1408
Changes from 19 commits
72af3a0
1f2b962
a54cd34
f198f3e
8748fcf
679f807
0f4d6c2
328c56d
f24e7d4
0f8695d
e2230b0
db26119
41e855b
f46e74c
8054ef6
e959ee5
d4d99bc
9dafafd
64c418c
06c1f79
d31a8f5
1074ed6
89f4c66
0dae0ff
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 |
---|---|---|
|
@@ -28,10 +28,16 @@ import { ResourceDetectionConfigWithLogger } from '../../../config'; | |
class AwsEc2Detector implements Detector { | ||
/** | ||
* See https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-identity-documents.html | ||
* for documentation about the AWS instance identity document endpoint. | ||
* for documentation about the AWS instance identity document | ||
* and standard of IDMSv2. | ||
*/ | ||
readonly AWS_INSTANCE_IDENTITY_DOCUMENT_URI = | ||
'http://169.254.169.254/latest/dynamic/instance-identity/document'; | ||
readonly AWS_IDMS_ENDPOINT = '169.254.169.254'; | ||
readonly AWS_INSTANCE_TOKEN_DOCUMENT_PATH = '/latest/api/token'; | ||
readonly AWS_INSTANCE_IDENTITY_DOCUMENT_PATH = | ||
'/latest/dynamic/instance-identity/document'; | ||
readonly AWS_INSTANCE_HOST_DOCUMENT_PATH = '/latest/meta-data/hostname'; | ||
readonly AWS_METADATA_TTL_HEADER = 'X-aws-ec2-metadata-token-ttl-seconds'; | ||
readonly AWS_METADATA_TOKEN_HEADER = 'X-aws-ec2-metadata-token'; | ||
|
||
/** | ||
* Attempts to connect and obtain an AWS instance Identity document. If the | ||
|
@@ -44,41 +50,86 @@ class AwsEc2Detector implements Detector { | |
*/ | ||
async detect(config: ResourceDetectionConfigWithLogger): Promise<Resource> { | ||
try { | ||
const token = await this._fetchToken(); | ||
const { | ||
accountId, | ||
instanceId, | ||
instanceType, | ||
region, | ||
availabilityZone, | ||
} = await this._awsMetadataAccessor(); | ||
} = await this._fetchIdentity(token); | ||
const hostname = await this._fetchHost(token); | ||
|
||
return new Resource({ | ||
[CLOUD_RESOURCE.PROVIDER]: 'aws', | ||
[CLOUD_RESOURCE.ACCOUNT_ID]: accountId, | ||
[CLOUD_RESOURCE.REGION]: region, | ||
[CLOUD_RESOURCE.ZONE]: availabilityZone, | ||
[HOST_RESOURCE.ID]: instanceId, | ||
[HOST_RESOURCE.TYPE]: instanceType, | ||
[HOST_RESOURCE.NAME]: hostname, | ||
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. Name and hostname are the same? 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. Yes, at least in Java implementation they take Name and hostname as the same: https://github.com/open-telemetry/opentelemetry-java/blob/master/sdk_extensions/aws_v1_support/src/main/java/io/opentelemetry/sdk/extensions/trace/aws/resource/Ec2Resource.java#L192 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. The main reason for this is not knowing how to populate this :) I filed an issue to ask about why there are two similar fields |
||
[HOST_RESOURCE.HOSTNAME]: hostname, | ||
}); | ||
} catch (e) { | ||
config.logger.debug(`AwsEc2Detector failed: ${e.message}`); | ||
return Resource.empty(); | ||
} | ||
} | ||
|
||
private async _fetchToken(): Promise<string> { | ||
const options = { | ||
host: this.AWS_IDMS_ENDPOINT, | ||
path: this.AWS_INSTANCE_TOKEN_DOCUMENT_PATH, | ||
method: 'PUT', | ||
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. Why is this 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. This is part of AWS IDMSv2 standard, we need to do |
||
timeout: 1000, | ||
headers: { | ||
[this.AWS_METADATA_TTL_HEADER]: '60', | ||
}, | ||
}; | ||
return await this._fetchString(options); | ||
} | ||
|
||
private async _fetchIdentity(token: string): Promise<any> { | ||
const options = { | ||
host: this.AWS_IDMS_ENDPOINT, | ||
path: this.AWS_INSTANCE_IDENTITY_DOCUMENT_PATH, | ||
method: 'GET', | ||
timeout: 1000, | ||
headers: { | ||
[this.AWS_METADATA_TOKEN_HEADER]: token, | ||
}, | ||
}; | ||
const identity = await this._fetchString(options); | ||
return JSON.parse(identity); | ||
} | ||
|
||
private async _fetchHost(token: string): Promise<string> { | ||
const options = { | ||
host: this.AWS_IDMS_ENDPOINT, | ||
path: this.AWS_INSTANCE_HOST_DOCUMENT_PATH, | ||
method: 'GET', | ||
timeout: 1000, | ||
obecny marked this conversation as resolved.
Show resolved
Hide resolved
|
||
headers: { | ||
[this.AWS_METADATA_TOKEN_HEADER]: token, | ||
}, | ||
}; | ||
return await this._fetchString(options); | ||
} | ||
|
||
/** | ||
* Establishes an HTTP connection to AWS instance identity document url. | ||
* Establishes an HTTP connection to AWS instance document url. | ||
* If the application is running on an EC2 instance, we should be able | ||
* to get back a valid JSON document. Parses that document and stores | ||
* the identity properties in a local map. | ||
*/ | ||
private async _awsMetadataAccessor<T>(): Promise<T> { | ||
private async _fetchString(options: http.RequestOptions): Promise<string> { | ||
return new Promise((resolve, reject) => { | ||
const timeoutId = setTimeout(() => { | ||
req.abort(); | ||
reject(new Error('EC2 metadata api request timed out.')); | ||
}, 1000); | ||
|
||
const req = http.get(this.AWS_INSTANCE_IDENTITY_DOCUMENT_URI, res => { | ||
const req = http.request(options, res => { | ||
clearTimeout(timeoutId); | ||
const { statusCode } = res; | ||
res.setEncoding('utf8'); | ||
|
@@ -87,7 +138,7 @@ class AwsEc2Detector implements Detector { | |
res.on('end', () => { | ||
if (statusCode && statusCode >= 200 && statusCode < 300) { | ||
try { | ||
resolve(JSON.parse(rawData)); | ||
resolve(rawData); | ||
} catch (e) { | ||
reject(e); | ||
} | ||
|
@@ -102,6 +153,7 @@ class AwsEc2Detector implements Detector { | |
clearTimeout(timeoutId); | ||
reject(err); | ||
}); | ||
req.end(); | ||
}); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe
AWS_IDMS_HOST_ADDRESS
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice suggestion!
But because we have another path called HOST_DOCUMENT. To avoid confusion, I suppose this kind of naming is also acceptable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's IMDS (Instance Metadata Service) ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for pointing out! Will modify this