-
Notifications
You must be signed in to change notification settings - Fork 889
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change golang namespace to 'go', rather than 'gc' #2262
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:) it has a GC :))
be9442d
to
91c912d
Compare
95addde
to
606b90a
Compare
As noted in my response to Tigran's question, I've learned that Go has a I've also added exact guidelines for populating |
cc: @open-telemetry/go-approvers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I cannot unresolve a comment 😄 Please just take a look at: #2262 (comment) (it is not a blocker for me) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Let's leave this PR hanging for couple of days to give chance other @open-telemetry/proto-go-approvers to comment
@open-telemetry/go-approvers @open-telemetry/go-maintainers please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not believe this change is necessary. This affects a value in an attribute that clearly does not relate to garbage collection. For attribute keys created by our runtime metrics instrumentation we already use runtime.go.*
. Having a special case for a particular compiler does not make sense to me here.
The initial intent of this PR was to support the The
Only after feedback did I take concern of the attribute values, and yet until now I did not recognize that my initial assumption about namespaces was wrong. Anyways, I see now that
I assume we still want to specify exactly how I think there is a case for renaming it |
That actually seems like a reason for not renaming it. If we made such a change users could know that any metric bearing That said, I think it is all irrelevant as the |
I'm happy for this change to go ahead too 👍🏻 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, "gc" is plan9-ism that probably few people in this community actually care about.
@Aneurysm9 how strong is your position? Please indicate if your comment "That actually seems like a reason for not renaming it." is blocking so we can try to find a consensus. |
If there is consensus that Again, not going to block if I'm the only one that feels that way, but I would like for those that approved prior to the comment @djaglowski made about misinterpreting the purpose of this file to re-assess their approvals. It seems that the PR was created for one purpose, realized to not be appropriate for that purpose, but maintained anyway. We should assure that it is being approved for the currently intended purpose. |
@Aneurysm9 if the variable in question were named |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Per the specification: open-telemetry/opentelemetry-specification#2262 Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Per the specification: open-telemetry/opentelemetry-specification#2262 Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Per the specification: open-telemetry/opentelemetry-specification#2262 Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
* Change golang namespace to 'go', rather than 'gc' * Specify common process.runtime.* attributes for Go * Add link to runtime.Compiler * Remove static descriptions * Add table of descriptions for golang runtime names * Add context to go examples Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com>
Resolves opentelemetry-go-contrib #350.
This was discussed briefly in the Golang SIG in December. Consensus at the time was that
go
is clearer thangc
and sufficiently appropriate. The move away fromgc
is helpful in part because the namespace is used to mean "garbage collector" elsewhere in the semantic conventions.Related issues #
opentelemetry-go-contrib #1456