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

AWS X-Ray remote sampler does not poll for new rules #376

Closed
felixscheinost opened this issue Jun 28, 2022 · 0 comments
Closed

AWS X-Ray remote sampler does not poll for new rules #376

felixscheinost opened this issue Jun 28, 2022 · 0 comments
Labels
component: aws-xray type: bug Something isn't working

Comments

@felixscheinost
Copy link
Contributor

Description

AWS X-Ray remote sampler does not poll for updated sampling configuration in the default configuration.

Any configuration which has a polling interval smaller than roughly 3.3 minutes is expected to work currently.

I already addeed a comment in #133 but decided to open a separate issue instead.

Steps to reproduce

Run in default configuration.

Expectation

I expect the sampling configuration to be fetched every 5 minutes.

What applicable config did you use?

AWS X-Ray OpenTelemetry distro Java agent with java option -Dotel.traces.sampler=xray

Relevant Environment Information

Version: 1.14.0 but main should be affecteed as well.
OS: macOS and AWS Fargate

Additional context

I configured the -Dotel.traces.sampler=xray on my development machine where it is expected to fail as I don't have any AWS proxy running locally but should still at least try every 5 minutes.

I set a breakpoint here

long delay = pollingIntervalNanos + RANDOM.nextInt(jitterNanos);

and noticed that in my case jitterNanos = -1294967296. This screams truncation error so I checked where jitterNanos is set. It is set here

so 1% of the pollingIntervalNanos but cast to int.

Default is pollingIntervalNanos = 300000000000 = 3 * 10^11, so pollingIntervalNanos / 100 = 3 * 10^9 which is bigger than Integer.MAX_VALUE = 2147483647 ~= 2.1 * 10^9

I see the reason why it is cast to int is because there is a method Random.nextInt(Int) but no method Random.nextLong(Long).

I think the easiest fix would be to store the polling interval in milliseconds instead. I don't think nanoseconds makes sense here.

@felixscheinost felixscheinost added the type: bug Something isn't working label Jun 28, 2022
felixscheinost added a commit to felixscheinost/opentelemetry-java-contrib that referenced this issue Jun 28, 2022
In the default configuration `pollingIntervalNanos = 3 * 10^11` so `pollingIntervalMillis / 100 > Integer.MAX_VALUE`.

Switch to storing everything in milliseconds instead. This should be a non-breaking change as all the nanoseconds related logic isn’t part of the public API.
felixscheinost added a commit to felixscheinost/opentelemetry-java-contrib that referenced this issue Jun 28, 2022
In the default configuration `pollingIntervalNanos = 3 * 10^11` so `pollingIntervalMillis / 100 > Integer.MAX_VALUE`.

Switch to storing everything in milliseconds instead. This should be a non-breaking change as all the nanoseconds related logic isn’t part of the public API.
felixscheinost added a commit to felixscheinost/opentelemetry-java-contrib that referenced this issue Jun 28, 2022
In the default configuration `pollingIntervalNanos = 3 * 10^11` so `pollingIntervalMillis / 100 > Integer.MAX_VALUE`.

Switch to storing everything in milliseconds instead. This should be a non-breaking change as all the nanoseconds related logic isn’t part of the public API.
felixscheinost added a commit to felixscheinost/opentelemetry-java-contrib that referenced this issue Jun 29, 2022
In the default configuration `pollingIntervalNanos = 3 * 10^11` so `pollingIntervalMillis / 100 > Integer.MAX_VALUE`.

Switch to storing everything in milliseconds instead. This should be a non-breaking change as all the nanoseconds related logic isn’t part of the public API.
felixscheinost added a commit to felixscheinost/opentelemetry-java-contrib that referenced this issue Jun 30, 2022
In the default configuration `pollingIntervalNanos = 3 * 10^11` so `pollingIntervalMillis / 100 > Integer.MAX_VALUE`.

Switch to storing the jitter in a `long` as well.
felixscheinost added a commit to felixscheinost/opentelemetry-java-contrib that referenced this issue Jun 30, 2022
In the default configuration `pollingIntervalNanos = 3 * 10^11` so `pollingIntervalMillis / 100 > Integer.MAX_VALUE`.

Switch to storing the jitter in a `long` as well.
felixscheinost added a commit to felixscheinost/opentelemetry-java-contrib that referenced this issue Jul 4, 2022
In the default configuration `pollingIntervalNanos = 3 * 10^11` so `pollingIntervalMillis / 100 > Integer.MAX_VALUE`.

Switch to storing the jitter in a `long` as well.
felixscheinost added a commit to felixscheinost/opentelemetry-java-contrib that referenced this issue Jul 4, 2022
In the default configuration `pollingIntervalNanos = 3 * 10^11` so `pollingIntervalMillis / 100 > Integer.MAX_VALUE`.

Switch to storing the jitter in a `long` as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: aws-xray type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants