Skip to content
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

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

AsuNa-jp
Copy link

@AsuNa-jp AsuNa-jp commented Oct 25, 2024

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

Copy link

github-actions bot commented Dec 4, 2024

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Dec 4, 2024
@AsuNa-jp AsuNa-jp changed the title add origin_referrer_url, origin_url and zone_identifier to the process attribute add origin_referrer_url, origin_url to the process attribute Dec 5, 2024
@AsuNa-jp AsuNa-jp requested a review from a team as a code owner December 5, 2024 05:28
@AsuNa-jp AsuNa-jp changed the title add origin_referrer_url, origin_url to the process attribute add origin_referrer_url and origin_url to the process attribute Dec 5, 2024
@braydonk
Copy link
Contributor

braydonk commented Dec 5, 2024

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 process.executable.origin_referrer_url and process.executable.origin_url.

@github-actions github-actions bot removed the Stale label Dec 6, 2024
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Dec 22, 2024
@trisch-me
Copy link
Contributor

@AsuNa-jp could you update this PR and address comments? thanks

@github-actions github-actions bot removed the Stale label Dec 24, 2024
@AsuNa-jp
Copy link
Author

Hi @braydonk @trisch-me
Sorry for the late reply and thank you for the feedback!
As per your suggestion, I have added origin_referrer_url and origin_url under the executable as shown below. If you have any other feedback, please feel free to share it.

  • process.origin_referrer_url to process.executable.origin_referrer_url
  • process.origin_url to process.executable.origin_url

6e52941

Copy link
Contributor

@braydonk braydonk left a 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.
Copy link
Contributor

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?

Copy link
Contributor

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

@AsuNa-jp
Copy link
Author

Hi @trisch-me
Who should I ask for a review/approval to get merged this PR merged? (It seems that I still need an approval from
@open-telemetry/specs-semconv-maintainers as follows.)
image

@trisch-me
Copy link
Contributor

@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

@AsuNa-jp
Copy link
Author

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

Copy link
Contributor

@lmolkova lmolkova left a 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:

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
Copy link
Contributor

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?

@AsuNa-jp
Copy link
Author

Hi @lmolkova
Thank you for your feedback on this PR. The documentation in the PR you are working on has been very helpful in understanding what needs to be considered when adding a new attribute.

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?

@github-actions github-actions bot added the enhancement New feature or request label Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

4 participants