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

Jump to bottom

fix: fix timeout in aws resource detector #2186

Merged
merged 12 commits into from
Jun 13, 2024
Merged

fix: fix timeout in aws resource detector #2186

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
496ab8f
fix: fix timeout in aws resource detector
arunp0 May 8, 2024
271b539
Merge branch 'main' into fix/aws-resource-detector-timeout
arunp0 May 9, 2024
3768701
Updated the timeouts as per comments
arunp0 May 12, 2024
9acdeac
Merge branch 'main' into fix/aws-resource-detector-timeout
arunp0 May 14, 2024
ad72fc2
Merge branch 'main' into fix/aws-resource-detector-timeout
arunp0 May 15, 2024
5277166
Merge branch 'main' into fix/aws-resource-detector-timeout
arunp0 May 16, 2024
84ddcf3
Merge branch 'main' into fix/aws-resource-detector-timeout
arunp0 May 17, 2024
0898fee
Merge branch 'main' into fix/aws-resource-detector-timeout
arunp0 May 18, 2024
bcc205b
Merge branch 'main' into fix/aws-resource-detector-timeout
arunp0 May 23, 2024
5b4c89c
Merge branch 'main' into fix/aws-resource-detector-timeout
arunp0 May 27, 2024
8363984
Merge branch 'main' into fix/aws-resource-detector-timeout
arunp0 Jun 2, 2024
e364811
Merge branch 'main' into fix/aws-resource-detector-timeout
pichlermarc Jun 13, 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
2 changes: 1 addition & 1 deletion 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 @@ -131,7 +131,7 @@ class AwsEc2Detector implements Detector {
const timeoutId = setTimeout(() => {
req.abort();
reject(new Error('EC2 metadata api request timed out.'));
}, 1000);
}, this.MILLISECOND_TIME_OUT);

const req = http.request(options, res => {
clearTimeout(timeoutId);
Expand Down
25 changes: 15 additions & 10 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 @@ -109,7 +109,8 @@ describe('awsEc2Detector', () => {
scope.done();
});

it('should throw when timed out', async () => {
it('should throw when timed out', function (done) {
this.timeout(7000);
Copy link
Contributor

Choose a reason for hiding this comment

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

if you're changing the timeout value for the test, you're not really testing the new value you changed to. Don't you want to test with the 5s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the timeouts

const expectedError = new Error('EC2 metadata api request timed out.');
const scope = nock(AWS_HOST)
.put(AWS_TOKEN_PATH)
Expand All @@ -120,17 +121,21 @@ describe('awsEc2Detector', () => {
.reply(200, () => mockedIdentityResponse)
.get(AWS_HOST_PATH)
.matchHeader(AWS_METADATA_TOKEN_HEADER, mockedTokenResponse)
.delayConnection(2000)
.delayConnection(6000)
.reply(200, () => mockedHostResponse);

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

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

it('should throw when replied with an Error', async () => {
Expand Down
Loading