Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDK should support an error callback #174

Closed
dmathieu opened this issue Oct 8, 2019 · 7 comments · Fixed by #791
Closed

SDK should support an error callback #174

dmathieu opened this issue Oct 8, 2019 · 7 comments · Fixed by #791
Assignees
Labels
area:metrics Part of OpenTelemetry Metrics area:trace Part of OpenTelemetry tracing pkg:SDK Related to an SDK package

Comments

@dmathieu
Copy link
Member

dmathieu commented Oct 8, 2019

As mentioned in #162 (comment)

@dmathieu dmathieu mentioned this issue Oct 8, 2019
@jmacd
Copy link
Contributor

jmacd commented Oct 8, 2019

The SDK itself also should support an OnError() callback so that it can issue errors to a handler configured by the application main() function. Probably every component (SDK, processor, exporter) should support this in a uniform way.

@akvanhar
Copy link
Contributor

akvanhar commented Oct 8, 2019

If this is something we expect exporters to handle in a uniform way, does it make sense to have it implemented here instead? A default OnError() call back could do log.Printf("Error when uploading spans to Exporter: %v", err). Even better if we know the name of the exporter in question.

@jmacd
Copy link
Contributor

jmacd commented Oct 10, 2019

Discussed in the weekly SIG meeting. We support the idea of an interface with an OnError method that is passed to the constructors of each SDK, processor, exporter, and plugin that might need to report errors.

@freeformz
Copy link
Contributor

@jmacd Do you mean an optional interface, such that the sdk checks to see if an exporter or the like is also an OnErrorer or the like and if so then uses this? Or something else?

@jmacd
Copy link
Contributor

jmacd commented Oct 15, 2019

To me there are two questions here.
Should each connected group of SDK/processor/exporter/plugin share a single OnError handler? (If yes, how is this organized?)
Should there be a global OnError handler? (If no, will there be conditions where there's not a well defined OnError handler?)
One approach would be to have the SDK implement an OnError handler, and let the pre-initialization global SDK have a sensible default. This is slightly complicated because the exporter and processors are going to be constructed before the SDK. Should we somehow make the OnError handler a (standard, functional) option passed to all components?

Another trouble with this idea is that we haven't specified a type for the "SDK" itself. At present all we have are the idea of a Tracer and Meter factory (for named tracers and meters) plus the standing OTEP 0005 about global initialization: https://github.com/open-telemetry/oteps/pull/45/files

@jmacd
Copy link
Contributor

jmacd commented Nov 20, 2019

I suggest this issue be named "SDK should to support an error callback". The current default metrics SDK supports one, but it should be supported by the tracer SDK as well.

@dmathieu dmathieu changed the title Exporters need to be able to register an optional error callback SDK should to support an error callback Nov 20, 2019
@jmacd jmacd added this to the Alpha v0.4 milestone Mar 7, 2020
@MrAlias MrAlias changed the title SDK should to support an error callback SDK should support an error callback Mar 20, 2020
vmihailenco added a commit to vmihailenco/opentelemetry-go that referenced this issue May 15, 2020
vmihailenco added a commit to vmihailenco/opentelemetry-go that referenced this issue May 15, 2020
@MrAlias MrAlias added area:metrics Part of OpenTelemetry Metrics pkg:SDK Related to an SDK package area:trace Part of OpenTelemetry tracing labels May 18, 2020
@MrAlias
Copy link
Contributor

MrAlias commented May 28, 2020

Possibly worth looking into adding a global error handler (to be useful prior to sdk registration).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics area:trace Part of OpenTelemetry tracing pkg:SDK Related to an SDK package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants