Hey @lalitb , happy to clean this one up, but first some questions:

• Do we want to return the status, or not?
• This looks like a breaking change either way. How do we handle that?

Feel free to assign it to me 💪

Following chat on CNCF slack, I'll pick this one up!

@scottgerring

Do we want to return the status, or not?

I vote for returning the status, so tracing and logs to be consistent with metrics. Also, good to see what specs says and other language implementation does.

This looks like a breaking change either way. How do we handle that?

We are still good with doing breaking changes with SDK. If it can be deprecated first while introducing new API that will be ideal.

@lalitb , thanks for the input 💪

I vote for returning the status, so tracing and logs to be consistent with metrics.

I agree; there's a couple situations where actual errors pop up in each of these cases that are being swallowed in the trace and logs impls. This is mainly to do with the acquisition of locks:

    /// If another user of this mutex panicked while holding the mutex, then
    /// this call will return an error once the mutex is acquired.

In the metrics case, we have e.g. this, where we are propagating the error:

One trace variant has the potential to hit a similar error, but this is swallowed:

... and the same for logs:

These are admittedly unlikely errors, but if they do come up something must be fairly wrong, and at least logging these would be helpful!

We are still good with doing breaking changes with SDK. If it can be deprecated first while introducing new API that will be ideal.

We could do this, but it would have to be a whole new method, and it'd be quite a sprawl through the code base, as we'd have to handle the variants down through the tree of clients too. You can get a feel for the scope of this on the PR I linked above.

As it's "only" a lint for users - "You should do something with this return value" - my gut feeling is that it'd be better to include a note about this in release notes, and pull the bandaid off. What do you think @lalitb ?

If it can be deprecated first while introducing new API that will be ideal.

It'll be hard to do that, as there is only one release (0.28) planned before stable, and we should not have breaking changes after 0.28... I am okay with breaking change for 0.28.