Chang timeout value to miliseconds

[mohammadVatandoost]

Fixes #2591

[codecov]

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 79.1%. Comparing base (2997c4b) to head (1479571).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
opentelemetry-otlp/src/exporter/http/mod.rs	0.0%	1 Missing ⚠️
opentelemetry-otlp/src/exporter/tonic/mod.rs	0.0%	1 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #2656   +/-   ##
=====================================
  Coverage   79.1%   79.1%           
=====================================
  Files        122     122           
  Lines      22562   22562           
=====================================
  Hits       17861   17861           
  Misses      4701    4701           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lalitb]

@mohammadVatandoost Thanks for working on this. Can you have a look into the OTLP flow, as I believe there are some other places too. E,g

opentelemetry-rust/opentelemetry-otlp/src/exporter/http/mod.rs

Lines 120 to 123 in 0592075

	.or(env::var(OTEL_EXPORTER_OTLP_TIMEOUT).ok())
	{
	Some(val) => match val.parse() {
	Ok(seconds) => Duration::from_secs(seconds),

.

Also update the changelog as this is a breaking change.

@cijothomas @TommyCpp - Should this be a breaking change for environment variables, or should we maintain the current behavior (interpreting values as seconds) while adding support for specifying milliseconds? For example, 10 (also 10s) would remain 10 seconds, while 10ms would represent 10 milliseconds. Currently, the compile-time configuration with_timeout accepts a duration argument, allowing both seconds and milliseconds. Trying to avoid breaking changes unless it significantly complicates the implementation, or adds to confusion.

[cijothomas]

@cijothomas @TommyCpp - Should this be a breaking change for environment variables, or should we maintain the current behavior (interpreting values as seconds) while adding support for specifying milliseconds? For example, 10 (also 10s) would remain 10 seconds, while 10ms would represent 10 milliseconds. Currently, the compile-time configuration with_timeout accepts a duration argument, allowing both seconds and milliseconds. Trying to avoid breaking changes unless it significantly complicates the implementation, or adds to confusion.

I think this is a bug and must be treated accordingly. Every bug can be thought of as a breaking change for those users who were relying on the buggy behavior 🤣 !
10s,10ms are not something covered by spec, and won't be respected by other languages, so its best to stick with ENV variables spec as-is.

I agree that the impact is bad - A user who had set 1 sec timeout now instantly only has a 1 ms timeout (that can easily be hit much more frequently) with the upgrade - this can be covered in change-log and migration guide as a key warning/bullet point.

Happy to hear other thoughts.

[TommyCpp]

10 (also 10s) would remain 10 seconds, while 10ms would represent 10 milliseconds

probably additional complexity we don't want to maintain on the long term. I think we should just note it in breaking changes

[mohammadVatandoost]

OTEL_EXPORTER_OTLP_TIMEOUT

@lalitb The line, you mentioned, is already changed. Please see the PR file changes.
I couldn't find any other places than these three files. Please let me know if i miss sth.

Changelog is updated

[lalitb]

I couldn't find any other places than these three files. Please let me know if i miss sth.

I think it looks good.

Changelog is updated

The changes also affects signal specific env-variables - OTEL_EXPORTER_OTLP_TRACES_TIMEOUT OTEL_EXPORTER_OTLP_METRICS_TIMEOUT OTEL_EXPORTER_OTLP_LOGS_TIMEOUT. Please mention them too in the changelog.

[lalitb]

LGTM, nit comment on updating changelog with signal specific env-variables.

[mohammadVatandoost]

The changes also affects signal specific env-variables - OTEL_EXPORTER_OTLP_TRACES_TIMEOUT OTEL_EXPORTER_OTLP_METRICS_TIMEOUT OTEL_EXPORTER_OTLP_LOGS_TIMEOUT. Please mention them too in the changelog.

Added