OTel Span is not set to status "ok" when ends successfully

[GFriedrich]

Hi,
I'm not sure if I'm missing something in my implementation, but I would expect that if an OTel span is ended and is yet "unset", it would be set to "ok".
Right now I can see it is always "unset" at the end which is somewhat odd.
But please let me know in case I've missed anything.
Thanks for looking at it in advance.

[jonatan-ivanov]

Thanks for the issue!
Are you interested in creating a PR to fix this?
OtelSpan has two end methods and the error method is already setting the status so setting in end should be straightforward.

[GFriedrich]

@jonatan-ivanov first of all: Thanks for checking.
Sure, I can create a PR with a fix, but it may take until the weekend.

[jonatan-ivanov]

That's totally fine, our next release is in two weeks. I asked since this seems a good issue if one wants to contribute. If you don't or you won't find the time, that's of course 120% fine, just let me know and I'll fix it.

[GFriedrich]

hey @jonatan-ivanov,
tried to fix the issue and found a few more issues, but let me start from the beginning:

• first tried to go with the most simple approach and set the status at the end of the span
  • but found that there is no getter for the status of the span, so you can't know if it was already set to ERROR and it could overwrite the status then to OK
• then thought about storing the status additionally on the OtelSpan, but this would've caused issues when creating the span by wrapping some existing span, as it suffers from the same issue: not knowing the previous status
• then thought about setting the span at the beginning, but this breaks setting the error status
  • because once it is set to OK, you can't change it anymore
• so finally went back to the original solution and now cast the the span to a ReadableSpan, which then generates the span data to get the status
  • not the most beautiful solution, but the best I came up with

Additionally I found the following other problems:

• on some error scenarios the error status was not set
  • fixed it in my PR because it made sense to me to create a PR which fixes the overall status of an OTel span
• some of the builder methods are returning a copy of the OtelSpan for no good reason
  • I could see that historically others were using the same solution before they were changed to return this
  • around the same time frame though some additional methods got added which copied this behaviour before it got fixed
  • so I think they all should be changed to return this
  • let me know if I should do this and then I would create some issue and a PR

[jonatan-ivanov]

some of the builder methods are returning a copy of the OtelSpan for no good reason

Let's do this in a separate PR.

[GFriedrich]

some of the builder methods are returning a copy of the OtelSpan for no good reason

Let's do this in a separate PR.

Have created a separate issue (#900) and PR (#901) for it.
Thanks @jonatan-ivanov 🤝