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

ec2 detector does not respect context timeout #5635

Open
OrHayat opened this issue May 22, 2024 · 3 comments
Open

ec2 detector does not respect context timeout #5635

OrHayat opened this issue May 22, 2024 · 3 comments
Labels
area: detector Related to a detector package bug Something isn't working detector: aws:ec2

Comments

@OrHayat
Copy link

OrHayat commented May 22, 2024

Description

ec2 detector does not respect context timeout and it wait ~30 seconds on non-ec2 machines(default)

Environment

any env that is not on ec2 machine

Steps To Reproduce

  1. Using this code snippet
    ctx, cancel := context.WithTimeout(context.Background(), time.Second*2)
    defer cancel()
    ctx:=context.WithTimeOut(ctx)
    ec2detector.NewResourceDetector().Detect(ctx)
  2. Run this code on any machine that is NOT in aws ec2

Expected behavior

expected the code to stop the detection attempt much much faster(2 seconds) but there issues in the ec2 detector code that dont allow this to work as expected

  1. it uses the soon to be deprecated aws v1 sdk that dont force you to pass context to aws operation(you can still but the code didnt did that)

  2. also if this will be fixed then there is issue in the opentelemtry resource.detect function(in the
    opentelemetry-go/sdk/resource package)
    that will mean that every detector that was specified after this detector when creating new resource will get context that expired because they dont run in parallel.
    but that issue is for the main open-telemetry repo

@bhaskarbanerjee
Copy link

Is the ec2 detector at all meant to run on non ec2 machines?

@MrAlias
Copy link
Contributor

MrAlias commented Jun 7, 2024

Is the ec2 detector at all meant to run on non ec2 machines?

No it is not. Though, it should fail gracefully when it is not running there. I think eating up all the context time-out is not idea.

@MrAlias
Copy link
Contributor

MrAlias commented Jun 7, 2024

We should determine the SLA for the AWS response time to the EC2 metadata endpoint. The detector should then create a child context that has that set as its timeout so the parent timeout isn't completely used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: detector Related to a detector package bug Something isn't working detector: aws:ec2
Projects
None yet
Development

No branches or pull requests

3 participants