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

Specify naming scheme of default service name #556

Merged
merged 2 commits into from
Dec 2, 2021

Conversation

felixbarny
Copy link
Member

@felixbarny felixbarny commented Nov 18, 2021

Also turns some SHOULDs into MUSTs

  • Create PR as draft
  • Approval by at least one other agent
  • Mark as Ready for Review (automatically requests reviews from all agents and PM via CODEOWNERS)
    • Remove PM from reviewers if impact on product is negligible
    • Remove agents from reviewers if the change is not relevant for them
  • Merge after 2 business days passed without objections

Relates to https://github.com/elastic/observability-design/issues/21
/cc @formgeist

@apmmachine
Copy link

apmmachine commented Nov 18, 2021

💚 Build Succeeded

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

Expand to view the summary

Build stats

  • Start Time: 2021-11-29T16:35:44.227+0000

  • Duration: 4 min 42 sec

  • Commit: 8612e09

Co-authored-by: Sergey Kleyman <SergeyKleyman@users.noreply.github.com>
@felixbarny felixbarny marked this pull request as ready for review November 29, 2021 16:36
@felixbarny felixbarny requested review from a team as code owners November 29, 2021 16:36
@felixbarny felixbarny removed the request for review from a team November 29, 2021 16:36
Copy link
Member

@trentm trentm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Just to say it out loud (and perhaps help searching), this will change the current unspec'd fallbacks of nodejs_service and python_service in the Node.js and Python agents.

Copy link
Contributor

@gregkalapos gregkalapos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,

similarly to what @trentm says - in .NET the fallback is unknown and we'll need to change that if this gets merged. (good thing the fallback is really and edge case).

Copy link
Member

@vigneshshanmugam vigneshshanmugam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, we will switch to unknown in the RUM agent. Right now RUM agent does not have zero config support, I have opened elastic/apm-agent-rum-js#1103 to address this.

@felixbarny
Copy link
Member Author

LGTM, we will switch to unknown in the RUM agent

For RUM, the default would be unknown-rum-js-service.

@felixbarny felixbarny linked an issue Dec 2, 2021 that may be closed by this pull request
@felixbarny felixbarny merged commit 34f54c7 into master Dec 2, 2021
@felixbarny felixbarny deleted the default-service-name branch December 2, 2021 07:13
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.

[META 571] Spec: Align on default service name
8 participants