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

Should span creation operations raise exceptions? #141

Closed
c24t opened this issue Jun 19, 2019 · 2 comments · Fixed by #153
Closed

Should span creation operations raise exceptions? #141

c24t opened this issue Jun 19, 2019 · 2 comments · Fixed by #153
Labels
area:api Cross language API specification issue
Milestone

Comments

@c24t
Copy link
Member

c24t commented Jun 19, 2019

From open-telemetry/opentelemetry-python#11 (review).

The java tracer API docs note that calling spanBuilder with a null spanName arg should raise a NullPointerException. NoopSpanBuilder does this, but SpanBuilderSdk doesn't.

Two questions here:

Should we create a span with a default name (as @reyang and @carlosalberto suggest) instead of raising if the span name is null here? If we want to avoid exceptions in the API altogether this suggests we should also create a blank span current if the user calls Tracer.withSpan with a null span, but this seems less defensible than using a default name.

More generally, which errors should implementations raise? I could imagine letting the implementation be either more or less restrictive than the API, but if it can both raise exceptions that aren't listed in the API and choose not to raise exceptions that are... then there doesn't seem to be much point to listing exceptions in the API.

@c24t c24t added the area:api Cross language API specification issue label Jun 19, 2019
@SergeyKanzhelev
Copy link
Member

does this document looks good to be moved to spec? https://github.com/open-telemetry/opentelemetry-dotnet/blob/master/docs/error-handling.md

I'm happy to generalize it

@SergeyKanzhelev SergeyKanzhelev added this to the API revision: 07-2019 milestone Jun 20, 2019
@c24t
Copy link
Member Author

c24t commented Jun 20, 2019

That looks great, especially

Open Census SDK must not throw or leak unhandled or user unhandled exceptions

We still need to decide whether span creation should use a default name instead of raising an error though. If we do this in the python client we'll break with the java impl now, so we need to either mention this in the spec or decide that it's up to each language API to define defaults like this.

@SergeyKanzhelev SergeyKanzhelev removed this from the API revision: 07-2019 milestone Sep 27, 2019
@bogdandrutu bogdandrutu added this to the Alpha v0.3 milestone Sep 30, 2019
TuckTuckFloof pushed a commit to TuckTuckFloof/opentelemetry-specification that referenced this issue Oct 15, 2020
rockb1017 pushed a commit to rockb1017/opentelemetry-specification that referenced this issue Nov 18, 2021
* Update configuration.md

Soften the normative requirement for all code to support environment variables to a recommendation. RUM libraries are already documented and specified to not support this configuration method, therefore there exist valid reasons why this part of the specification will not be implementable and should be a recommendation.

* Update with conditional requirement
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Cross language API specification issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants