open-telemetry / opentelemetry-erlang-api

Erlang/Elixir OpenTelemetry API
Apache License 2.0
60 stars 13 forks source link

Updated Readme, added `:opentelemetry` into extra_applications. #63

Closed yatender-oktalk closed 4 years ago

yatender-oktalk commented 4 years ago

Changes to be committed: modified: README.md

tsloughter commented 4 years ago

I don't think this is right. Why are you adding the SDK to extra_applications?

yatender-oktalk commented 4 years ago

I don't think this is right. Why are you adding the SDK to extra_applications?

Hey, when we don't add :opentelemetry to extra_applications, the span doesn't start, when we run the following commands.

require OpenTelemetry.Tracer OpenTelemetry.Tracer.start_span("span_name") {:span_ctx, 0, 0, 0, [], false, :undefined, false}

When we just use dependency opentelemetry in mix.exs instead of opentelemetry_api everything works fine.

When we only add opentelemetry_api then we have to add :opentelemetry in extra_applications.

tsloughter commented 4 years ago

Ah, ok. So the issue is the readme isn't clear on the separation between the API and SDK and how to use them then.

The API package should be used and included without the SDK in Applications that are published on their own. Like if someone were to add opentelemetry spans to a database library directly or added instrumentation by making a middleware for an http server. In those cases only the API should be included -- though you might include the SDK in a test profile so that you can test the spans and what not work.

When you have a project that will actually run on its own, like a release, you then need to include the SDK as a dependency.

This keeps the API library small and allows developers to include it in their applications even if a user might not want to use it. When no SDK is included it won't do anything and thus the developer doesn't have to worry about those end users who want to use something else or not enable OpenTelemetry for whatever reason.

yatender-oktalk commented 4 years ago

Oh got it. 👍

yatender-oktalk commented 4 years ago

We can close this PR then?

tsloughter commented 4 years ago

Yea. Unless you want to update it to cover what I mentioned, but feel free to close this PR and I'll try to get to updating the readme and docs soon.