jjatria / perl-opentelemetry-sdk

An implementation of the OpenTelemetry SDK for Perl
https://metacpan.org/pod/OpenTelemetry::SDK
Other
9 stars 4 forks source link

default config with otlp exporter silently fails #18

Open abraxxa opened 1 month ago

abraxxa commented 1 month ago

Reading the comments of https://github.com/jjatria/perl-opentelemetry-sdk/issues/5#issuecomment-1821724308 and adding use Log::Any::Adapter 'Stderr'; use Metrics::Any::Adapter 'Stderr'; to my code showed that OpenTelemetry::Exporter::OTLP can't be loaded because it's a separate CPAN dist: Error configuring 'otlp' span processor: Can't locate OpenTelemetry/Exporter/OTLP.pm in @INC (you may need to install the OpenTelemetry::Exporter::OTLP module) (@INC entries checked: lib /home/ahartmai/.plenv/versions/5.40.0/lib/perl5/site_perl/5.40.0/x86_64-linux /home/ahartmai/.plenv/versions/5.40.0/lib/perl5/site_perl/5.40.0 /home/ahartmai/.plenv/versions/5.40.0/lib/perl5/5.40.0/x86_64-linux /home/ahartmai/.plenv/versions/5.40.0/lib/perl5/5.40.0) at /home/ahartmai/.plenv/versions/5.40.0/lib/perl5/site_perl/5.40.0/Module/Runtime.pm line 314.

OpenTelemetry::SDK should at least warn if not die with that message.

The documentation really needs a Getting started section too...

jjatria commented 6 days ago

Thanks for bringing this up. A "Getting started" or some sort of cookbook would be super helpful, and it is sorely missing.

OpenTelemetry::SDK should at least warn if not die with that message

This bit in the specification is probably relevant here: https://opentelemetry.io/docs/specs/otel/error-handling/#basic-error-handling-principles

Basic error handling principles

OpenTelemetry implementations MUST NOT throw unhandled exceptions at run time.

[...]

  1. The API or SDK MAY fail fast and cause the application to fail on initialization, e.g. because of a bad user config or environment, but MUST NOT cause the application to fail later at run time, e.g. due to dynamic config settings received from the Collector.

[...]

This could be a case where we want to "fail fast" because of misconfiguration. In general, I've tried to err on the side of "application stability is more important than telemetry, so your application should never die because of OpenTelemetry", but this would not actually go against the spec. We could make this configurable, with some sort of Perl-specific OTEL_PERL_FAIL_FAST variable or something along those lines.

The silent logging bit is annoying, but it could be that for internal or specific issues we want to do something like

-$logger->warnf("Error configuring '%s' propagator: %s", $name, $e);
+warn $logger->warnf("Error configuring '%s' propagator: %s", $name, $e);

and both emit a log and raise a warning. The problem there is that applications that are using eg. a Stderr log adapter will get that line twice.

abraxxa commented 6 days ago

I see... So the exporter(s) are configured (using env vars) but can't be changed after startup, right? If this is the case I'd opt for fail fast at startup.

As for the logging of such fast fail messages I'd not use any logger for them but a regular die.