[exporter/datadogexporter] Rely on http.Client's timeout instead of in exporterhelper's

[mx-psi]

Description:

Rely on http.Client timeout instead of exporterhelper's.

Since exporterhelper's applies to the whole push[Trace/Metric]Data call and we do several network requests per call, it is preferable to do it this way to prevent one network call to cause timeouts in the next one.

This is also required for #6412, since retries inside the push functions increase the time taken.

Link to tracking Issue: n/a

Testing: Things to test manually

• Check that timeout does occur on network calls.

Documentation: none, this should be transparent to the user.

[KSerrania]

Left a question, LGTM

[KSerrania]

This changes the timeout value from 10 seconds to 15 seconds (if I read

opentelemetry-collector-contrib/exporter/datadogexporter/factory.go

Lines 49 to 53 in 77d48a5

	func defaulttimeoutSettings() exporterhelper.TimeoutSettings {
	return exporterhelper.TimeoutSettings{
	Timeout: 15 * time.Second,
	}
	}

correctly), is that fine? Should we change defaulttimeoutSettings to return 10 seconds instead?

[mx-psi]

Yes, this is fine (we care about the timeout of the batch processor for traces but that's about it, the timeout here should be fine no matter the value)

[mx-psi]

@gbbr could you review?

[mx-psi]

/easycla

[codeboten]

Overall the PR looks good, just one question before approving.

[codeboten]

Will this be confusing if some exporters use this TimeoutSettings as the timeout setting for the entire operation where as other exporters use it per network call? This makes me think we should have a different configuration option for it. What do you think?

[mx-psi]

I have tried to use a pattern that is already used in the Collector in other exporters:

opentelemetry-collector-contrib/exporter/signalfxexporter/factory.go

Lines 121 to 122 in 243d642

	// explicitly disable since we rely on http.Client timeout logic.
	exporterhelper.WithTimeout(exporterhelper.TimeoutSettings{Timeout: 0}),

opentelemetry-collector-contrib/exporter/lokiexporter/factory.go

Lines 71 to 72 in 243d642

	// explicitly disable since we rely on http.Client timeout logic.
	exporterhelper.WithTimeout(exporterhelper.TimeoutSettings{Timeout: 0}),

opentelemetry-collector-contrib/exporter/splunkhecexporter/factory.go

Lines 105 to 106 in 243d642

	// explicitly disable since we rely on http.Client timeout logic.
	exporterhelper.WithTimeout(exporterhelper.TimeoutSettings{Timeout: 0}),

opentelemetry-collector-contrib/exporter/observiqexporter/exporter.go

Lines 42 to 43 in 243d642

	// explicitly disable since we rely on http.Client timeout logic.
	exporterhelper.WithTimeout(exporterhelper.TimeoutSettings{Timeout: 0}),

I would expect most users not to care about a global Consume[Metrics/Traces/Logs] function timeout (they don't even need to know that such a function exists to use the exporter). If we use different options I think we should have a wider conversation to have a consistent solution for all exporters that do this differently today.

[codeboten]

Agreed, I'm happy to approve this PR. It would be good to have that discussion if the pattern of disabling this timeout already happening, maybe the global timeout is less important.

[mx-psi]

Opened open-telemetry/opentelemetry-collector#4497 to discuss this