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

Populate event.dataset based on service.name #29

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tobiasstadler
Copy link

ecs-logging-java does a similar thing

@apmmachine
Copy link
Contributor

apmmachine commented Mar 24, 2021

❕ Build Aborted

The PR is not allowed to run in the CI yet

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Start Time: 2023-02-14T19:43:18.209+0000

  • Duration: 0 min 27 sec

Steps errors 2

Expand to view the steps failures

Load a resource file from a library
  • Took 0 min 0 sec . View more details here
  • Description: approval-list/elastic/ecs-logging-php.yml
Error signal
  • Took 0 min 0 sec . View more details here
  • Description: githubApiCall: The REST API call https://api.github.com/orgs/elastic/members/tobiasstadler return the message : java.lang.Exception: httpRequest: Failure connecting to the service https://api.github.com/orgs/elastic/members/tobiasstadler : httpRequest: Failure connecting to the service https://api.github.com/orgs/elastic/members/tobiasstadler : Code: 404Error: {"message":"User does not exist or is not a member of the organization","documentation_url":"https://docs.github.com/rest/reference/orgs#check-organization-membership-for-a-user"}

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@pimjansen
Copy link
Contributor

pimjansen commented Mar 24, 2021

This is weird imo? You log it to the service space right? Why push it to the event in that case

@tobiasstadler
Copy link
Author

By default Logs app in Kibana displays the event.dataset field (see https://www.elastic.co/guide/en/kibana/current/observability.html#logs-app), so I think it is valuable to have support for it in the php Ed’s logging implementation. The Java implementation populates the field based on the service name, if not explicitly given (see https://github.com/elastic/ecs-logging-java/blob/09c07cf451bebb86f82e3f21abc0579455d81ee8/ecs-logging-core/src/main/java/co/elastic/logging/EcsJsonSerializer.java#L283) so I thought we should do the same here.

Do you think creating a Event class is better?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants