Skip to content

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

Closed
@GFriedrich

Description

@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.

Activity

added this to the 1.3.7 milestone on Nov 21, 2024
jonatan-ivanov

jonatan-ivanov commented on Nov 21, 2024

@jonatan-ivanov
Member

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

GFriedrich commented on Nov 21, 2024

@GFriedrich
ContributorAuthor

@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

jonatan-ivanov commented on Nov 22, 2024

@jonatan-ivanov
Member

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.

added a commit that references this issue on Nov 24, 2024
ad94ae3
GFriedrich

GFriedrich commented on Nov 24, 2024

@GFriedrich
ContributorAuthor

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
added a commit that references this issue on Nov 24, 2024
0d7fe5b
added 3 commits that reference this issue on Dec 6, 2024
3cc1825
8ecd505
a0b3064

5 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugA general bug

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      OTel Span is not set to status "ok" when ends successfully · Issue #890 · micrometer-metrics/tracing