Add semantic conventions for thread.name and thread.id span attributes

[mateuszrzeszutek]

Fixes #788

Changes

Added two new span attributes responsible for storing current thread information: thread.name and thread.id

Related issues: open-telemetry/opentelemetry-java-instrumentation#942

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Mateusz Rzeszutek (9b2de21, 2400112)

[bogdandrutu]

Please sign the CLA

[tsloughter]

So this is for threads that might not necessarily be 1:1 with OS threads?

I'm wondering if there is a term that could be used that isn't "thread" or "process" so this can be used for (without being confusing) for both.

Specifically thinking of the need to add Erlang process pid's (and registered names) as attributes. Though Erlang process id's are made up of 3 integers.. so this might not work any way.

[kenfinnigan]

Also bear in mind that with Java there will soon be Fibers as well.

[mateuszrzeszutek]

So this is for threads that might not necessarily be 1:1 with OS threads?

As @Oberon00 mentioned here, info about the managed thread seems to be the most interesting when you want to debug what exactly happened in you application.

Also bear in mind that with Java there will soon be Fibers as well.

Most likely we'll want to switch to Strand.currentStrand() then.

[tsloughter]

@mateuszrzeszutek if this is meant to be the thread that started the span then it shouldn't be Strand.currentStrand(). Unless my limited understanding of Fibers is wrong. A span doesn't run across Fibers, unlike how it could with the underlying threads. So if this is to record the OS thread that started the span it would be unrelated to the current strand (when fibers are being used).

[mateuszrzeszutek]

if this is meant to be the thread that started the span then it shouldn't be Strand.currentStrand()

I was under the impression that we might want to store information about a Fiber that's running (starting) a span. Though now that I think about it, it might be better to store that information it in addition to the thread info, not instead.

To sum up: in Java thread.* will store information about a Thread that starts a span.

[tsloughter]

Yes, I think if this semantic convention is merged that it should not include things like the Fiber id.

For Fibers and other user/language space forms of threads/processes (goroutine, Erlang process, etc) there should be a separate convention. I don't know if there should attempt to be a shared convention or if each should just do their own, like for Erlang, erlang.pid and erlang.registered_name, and for Java java.fiber.id -- not sure if goroutines provide access to their "id" that would be used here and don't think they can be named?

[anuraaga]

Is it worth adding the word OS, as in OS thread to this spec? I feel as any ID that's idiomatic to the user's program, OS or fiber or otherwise, could be ok here so maybe we've overdefined by adding implementation details. But if we want to corner in on OS threads specifically in this spec we could be more direct.

[tsloughter]

True. +1 to naming it os_thread.id or os.thread.id.

[Oberon00]

I would also prefer using just thread and not process.thread, as I said above. I do think it's mostly a matter of taste though.

[mateuszrzeszutek]

I don't have a strong preference for either of these, both process.thread.* and thread.* are fine (and have their own respective advantages).
@tigrannajaryan @yurishkuro can I rely on you for the final judgement regarding the process. prefix?

[tsloughter]

@iNikem na, I get that there shouldn't technically be a rule against it but I still think process. makes more sense for simply the attributes (that happen to be in a resource) about the running os process.

[tigrannajaryan]

When I suggested process.* as the namespace I was thinking that since the thread is part of a process (is it possible for a thread not to be?) then it is reasonable to place it inside the process namespace.

I was wrong, I now think this is not a valid reason for choosing a namespace. The fact that an entity is located within another entity is not a reason for forming nested namespaces. That is not the purpose of namespaces. The purpose of a namespace is to avoid name clashes. You place something in a namespace if that same name can exist elsewhere but have a different meaning. My understanding is that threads as we define them can only exist inside processes, there are no other threads that we need to differentiate from process threads. This means there is no need to place threads inside the process namespace.

I retract my earlier suggestion to use the "process" namespace for threads.

[mateuszrzeszutek]

Okay, I removed my last commit: that leaves us with thread.id and thread.name attributes.

[carlosalberto]

Merging as this issue was fully resolved and the changes are relatively trivial.

@yurishkuro please do a follow up if you think there's any pending concern still.

[johantiden]

Thread.currentThread().getId() is deprecated since java 19. There is a Thread.currentThread().threadId() instead.

[trask]

hi @johantiden! this page has been moved to another repo: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/attributes.md#general-thread-attributes

can you open an issue (or PR) over there? thanks

[arminru]

I opened an issue in open-telemetry/semantic-conventions#1375 so we don't miss it as this PR here was closed 4 years ago.

[johantiden]

Thanks for the pointers! I just called it as I saw it. I came to this commit from the original pull request.