-
Notifications
You must be signed in to change notification settings - Fork 189
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
add origin_referrer_url
and origin_url
to the process attribute
#1517
base: main
Are you sure you want to change the base?
add origin_referrer_url
and origin_url
to the process attribute
#1517
Conversation
Co-authored-by: Alexandra Konrad <[email protected]>
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
origin_referrer_url
, origin_url
and zone_identifier
to the process attributeorigin_referrer_url
, origin_url
to the process attribute
origin_referrer_url
, origin_url
to the process attributeorigin_referrer_url
and origin_url
to the process attribute
Since this is a property of the process's executable, and not of a process itself, I think it makes more sense for these attributes to be |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
@AsuNa-jp could you update this PR and address comments? thanks |
Hi @braydonk @trisch-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.
LGTM
brief: > | ||
The URL of the webpage that linked to the process's executable file. | ||
note: > | ||
This information comes from metadata or alternate data streams linked to the process's executable file. |
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.
Non-blocking, can be merged without resolving this unless there's a good answer.
@open-telemetry/specs-semconv-maintainers is there a better way to specify a note like this in the YAML? This results in two separate identical notes being generated in the markdown. A way to make one note about this that could be linked by both attributes would be nice, but as far as I know there isn't a way to do that right?
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.
there is no way to do this currently afaik, because we are not linking notes, it happens automatically
Hi @trisch-me |
@AsuNa-jp we have discussed yesterday that this PR is ready to be merged so we should just wait until one of the maintainers will merge it unless other questions will arise |
@trisch-me I am really glad to hear that! Thank you so much for your kind support! |
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'm blocking this since:
-
it uses attribute names that are defined in add
origin_referrer_url
andorigin_url
to the file attribute #1430, but that one is not merged. I recommend to collaborate on addorigin_referrer_url
andorigin_url
to the file attribute #1430 and reach conclusion there. -
it defines attributes that are not referenced by any semantic conventions, i.e. it's not clear who/how/when would populate them. Please consider adding events that would leverage these attributes or check if those are applicable to existing semantic conventions and reference these attributed from them.
We're writing down existing review practices in this PR #1707
component: process | ||
|
||
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
note: add process.executable.origin_referrer_url and process.executable.origin_url |
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.
are those necessary - could similar file
attributes being introduced in #1430 be used instead? if not, why?
Hi @lmolkova I now understand that adding a new attribute would be difficult without clearly defining who will use it. I understand that adding a new attribute would be difficult without clearly defining who will use it. This might be repeating a question I asked you the other day, but I initially thought that OS event processing was also possible with Otel. However, based on your feedback, I now understand that this is not the case. If possible, could you please share specific technical documentation or an example of Otel instrumentation that illustrates who uses Otel and how it is used? and if you have any examples of handling file or process-related events with Otel, could you share them with me? |
Changes
This PR adds the following attributes. (Updated Dec/25/2024)
process.executable.origin_referrer_url
process.executable.origin_url
Details
Generally, a process is generated from an executable file. Therefore, the process model naturally includes attributes containing information about the originating executable file. In this PR, we are adding the following two attributes to further extend the originating executable file details.
process.executable.origin_referrer_url
process.executable.origin_url
The name of these attributes and where to add it were decided after consulting with @joe-desimone and @trisch-me.
The meaning of each field is explained in detail in the PR below.
Merge requirement checklist
[chore]