search

open-telemetry / opentelemetry-php

The OpenTelemetry PHP Library
https://opentelemetry.io/docs/instrumentation/php/
Apache License 2.0
686 stars 170 forks source link

How to configure timeput and retry of exporter #1317

Open iosifch opened 1 month ago

iosifch commented 1 month ago

Hi! I see that there's this env variable, OTEL_EXPORTER_OTLP_TRACES_TIMEOUT, through which I can configure the timeout of the trace exporter, but what about the number of retries? Is there any way to configure the number of retries? Thanks

brettmc commented 1 month ago

Hi @iosifch it's not currently configurable, and I think that's because it isn't mentioned in the spec. Retry is mentioned in https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md but nothing about number of times to retry that I can see. We usually refer to a more mature implementation, and https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk-extensions/autoconfigure/README.md#otlp-exporter-retry talks about #retries, but specifically mentions that there is currently no way to customize.

So, I think the answer is that if it's important to folks, we could implement our own env var to control it, something like OTEL_PHP_EXPORTER_OTLP_TRACES_RETRIES (and/or OTEL_PHP_EXPORTER_OTLP_TIMEOUT to control all signals). I suspect the spec maintainers won't want to add an official OTEL_OTLP_TRACES_RETRIES, however there's a file-based configuration in progress, which offers a lot more configurability and that might be a good place to start.

iosifch commented 1 month ago

Hi, @brettmc! First of all, thank you for your response. Today I learned that the PHP SDK ignores the env variable OTEL_EXPORTER_OTLP_TRACES_TIMEOUT. So, what I understand now is that there's no way to control how data is send out from application. Isn't this a weakness, as large timeouts and retries may impact the application's performance? Most likely I'm missing something. What do you think, @brettmc?

iosifch commented 1 month ago

Oh, OTEL_EXPORTER_OTLP_TRACES_TIMEOUT will be taken in consideration starting with a future version :)

davidpocar commented 3 weeks ago

@brettmc Hello! I have a question regarding the timeout issue. I noticed this PR and this commit addressing the matter. Could you please let me know if you have any idea when these changes might be released? Thank you!

brettmc commented 2 weeks ago

@davidpocar sorry, I've been on vacation for a couple of weeks. We're getting close to the next release, I'm going to estimate about a week or two.

davidpocar commented 2 weeks ago

@brettmc Hello, hope the vacation went well. That is good news, looking forward to that. Also, sorry for being slightly off topic, but are there any plans for making the exporter asynchronous? Thanks!

brettmc commented 2 weeks ago

Also, sorry for being slightly off topic, but are there any plans for making the exporter asynchronous?

Yes, but not immediate ones. Part of the desire to drop 7.4+8.0 support recently was to make async exporting possible. There's a good prototype over in https://github.com/Nevay/opentelemetry-async-sdk which uses revolt, and we might try to base a future enhancement off.

brettmc commented 2 weeks ago

Back on-topic, I submitted https://github.com/open-telemetry/opentelemetry-configuration/pull/97 to allow configuring retry logic for exporting. If that is accepted, it should be a trivial change at our end to support it (we have all the variables, just not a source to set them from)

iosifch commented 2 weeks ago

Back on-topic, I submitted open-telemetry/opentelemetry-configuration#97 to allow configuring retry logic for exporting. If that is accepted, it should be a trivial change at our end to support it (we have all the variables, just not a source to set them from)

This one is quite nice