search

open-telemetry / opentelemetry-erlang-api

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

API changes again? #73

Closed tsloughter closed 3 years ago

tsloughter commented 3 years ago

I noticed today that based on the spec we technically should have start_span and start_as_current_span, instead of having start_inactive_span.

Python also went with use_span instead of set_span. I'm not sure if that would make more sense to @garthk than set_span which has caused confusion? Though they also have a set_span_in_context. https://opentelemetry-python.readthedocs.io/en/stable/api/trace.html

Anyway, I don't want to just keep breaking the API, even if we are pre 1.0, so wanted to see what people thought first. I think if we do do any changes right now we should try to think about all of them and do it in one go. The API spec is pretty stable and GA is coming up at this point so we shouldn't have to worry about any changes happening there causing us to have to change the API.

In this discussion and changes we should also finalize how context is working.

garthk commented 3 years ago

Thanks for splitting that out of the mess of #26!

I feel the 😬 on making breaking changes, for sure. You ok to add deprecated attributes to anything we're worried about in patches on the current release train, and start a big batch in an 1.0.0-rc.1 version?

tsloughter commented 3 years ago

I don't know. I feel like if we want to replace function names we should just do it and not do deprecation stuff. We obv would if it was after 1.0 but that is a case where you deprecate something, leave it for the release or two and then delete it. We should delete right away so updates needed to docs and code are caught now and not missed because people ignored deprecation warnings.

tsloughter commented 3 years ago

I guess the pain is not everyone runs xref or dialyzer and will thus only discover the changes when the code crashes...