$timout in PsrTransportFactory is not used

[xosofox]

Check https://github.com/open-telemetry/opentelemetry-php/blob/73ff5adcb8f1db348bedb422de760e475df16841/src/SDK/Common/Export/Http/PsrTransportFactory.php#L41

The create function has a parameter $timeout.

However, this value is not used anymore further - thus rendering the expectation of passing a timeout to have any effect useless.

Is there a reason why PsrTransport does not have this paramter?

I'm willing to contribute to fix this - as my app is hanging terribly in case the metrics server is not available - however I would like to understand any considerations to be taken into account

Or am I missing something? Is there another way to pass a timout to my ClientInterface?

[brettmc]

The timeout param is there because the spec says it should be: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.24.0/specification/performance.md#shutdown-and-explicit-flushing-could-block And, it doesn't work because we code to the PSR http client/request interfaces, and I believe there is not a way to do something useful with that timeout value, either as part of the request or the client. Timeouts do work for some transports though (notably, gRPC).

The only way I'm aware of to set a timeout is to pre-configure your http client of choice before giving it to the PsrTransportFactory.

[xosofox]

Thanks for getting back that quickly

I will try to investigate around Psr...

In the meantime, can you give me a pointer on how to pre-configure the Client when I'm useing this scenario:

    $transport = (new OtlpHttpTransportFactory())->create(.........);

How would that have to be expanded?

[brettmc]

Today, that scenario won't work, and you'd need to instantiate a PsrTransport directly rather than use the factory. With guzzle and symfony's client, it's an option in the Client constructor.

So, something like:

$client = new HttpClient(['timeout' => 1.5]); //symfony
$client = new Client(['timeout' => 1.5]); //guzzle

$transport = new PsrTransport($client, /* other params */);

But I also note from symfony's docs that you can set default_socket_timeout and that will be used as the default http request timeout, which I think means you could use Psr18 discovery and hence the OtlpHttpTransportFactory.

All that said, I think that OtlpHttpTransportFactory could accept an http client as a constructor argument, and only use discovery as a fallback. I don't think that it helps your use-case though...

[xosofox]

I worked around it by using the PsrTransportFactorydirectly, configuring my Client first.

OtlpHttpTransportFactory does not accept any parameters, it is relying on "::discover" - that could theoretically be improved, no?

[brettmc]

that could theoretically be improved, no

Yes. It could use ::discover only if a client was not provided already.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[holgerlausen]

Have experienced the fallout of the missing timeout implementation when the opentelemetry collector hangs as well...

@xosofox could you share sample code on how you worked around this?

[xosofox]

It's been a while and I am not 100% sure it works, but here is some of the code I used.

In a Symfony environment, I used DI to get some of the services, including a "ClientInterface" that has a preset timeout (in our case it was Guzzle)

// as constructor params: 
__construct (....
        private ClientInterface $client,
        private RequestFactoryInterface $requestFactory,
        private StreamFactoryInterface $streamFactory,
...)

// in the method that sends a  Span
...
        $factory = new PsrTransportFactory($this->client, $this->requestFactory, $this->streamFactory);

        $transport = $factory->create(
            $this->otlpEndpoint.'/v1/traces',
            'application/x-protobuf', //  'application/json',
            [],
            null,
            1, // not being used anyway, https://github.com/open-telemetry/opentelemetry-php/issues/1111
            0,
            0
        );

        /* @phpstan-ignore-next-line for some reason, phpstan won't accept the transport as TransportInterface */
        $exporter = new SpanExporter($transport);

        $tracerProvider = new TracerProvider(
            new SimpleSpanProcessor($exporter)
        );

        $this->tracer = $tracerProvider->getTracer('my-tracer-id');
...

Hope this helps a bit?

[brettmc]

Have experienced the fallout of the missing timeout implementation when the opentelemetry collector hangs as well...

@holgerlausen I'm really interested in what happened there to cause the collector to hang? Is there an issue raised for it? We recommend using the collector to avoid/minimize transport delays, so I'd like to understand it better.

[brettmc]

I don't believe there's a way to "discover" a psr-18 client with configuration, and nor is there a way to add options to psr-18's sendRequest method.

Perhaps we could consider getting rid of "bring your own http client" and instead choose one that we like (symfony, guzzle, ...), so that we have full control over its setup. I think this would also allow us to implement support for CA certs, keys, etc. :thinking:

[xosofox]

I'm not sure about @holgerlausen 's case, but no matter if the collector "hangs" or is simpliy unavailable for some reason, the timeout becomes important.

However, PSR itself is quite ignorant about the timeout and from my understanding, there is no real "place" for it. It's just being passed around as some option, which is IMHO not giving it the priority it deserves