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

Document common apm.getServiceName() API #404

Closed
wants to merge 1 commit into from

Conversation

trentm
Copy link
Member

@trentm trentm commented Jan 20, 2021

For ecs-logging implementations to populate the "service.name" and "event.dataset" fields they will need some way to get the service name from the detected APM Agent. I'm proposing a apm.getServiceName() -> String API for that, at least for the Node.js Agent.

I didn't see a current doc in apm.git/specs/agents/ that might appropriately hold this. I'm happy to take advice/opinions that documenting commonalities in the APM Agent APIs isn't time well spent.

The ecs-logging case for getServiceName() affects the Agent languages with ecs-loggers: https://github.com/elastic/ecs-logging#loggers In particular this does not include the RUM agent.

updating

Has a use case for a apm.getServiceName():

  • Node.js: yes
  • Go: ?
  • Java: no (instruments logging from APM side)
  • .NET: ?
  • Python: no (instruments logging from APM side)
  • Ruby: ?
  • PHP: ?
  • RUM JS: N/A

@trentm trentm self-assigned this Jan 20, 2021
@trentm
Copy link
Member Author

trentm commented Jan 20, 2021

It is possible to call this API with a not-yet-configured or mis-configured APM Agent such that there isn't a valid service name. This spec doesn't yet address how that should be handled. I can imagine the API for that would vary by language.

@apmmachine
Copy link

apmmachine commented Jan 20, 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

Expand to view the summary

Build stats

  • Build Cause: Started by timer

    • Start Time: 2021-01-25T04:42:00.188+0000
  • Duration: 13 min 57 sec

  • Commit: 1ad7f2b

@basepi
Copy link
Contributor

basepi commented Jan 20, 2021

Python's config parser will hard fail if there isn't a service name. We currently just get it from our config object, it would be fairly trivial to provide a public API call for it. Here is my implementation for Python: elastic/apm-agent-python#1014

@trentm
Copy link
Member Author

trentm commented Jan 21, 2021

@basepi Oh, interesting. IIUC, the addition of tracing-related logging fields is handled from the APM Agent side. (In Node.js at least, it is handled from the logging side: the logger will look for an active APM Agent and get trace data from it to log.) So for Python there isn't really a use case for apm.getServiceName().

Here is my implementation for Python: elastic/apm-agent-python#1014

"service.name" is not added if there is a log statement when there is no active transaction, then correct?

@felixbarny
Copy link
Member

Another option is that the agent instruments the ECS logger library to set the service name if it's empty.

- RUM JS: the singleton [Agent instance](https://www.elastic.co/guide/en/apm/agent/js-base/current/agent-api.html)


### `apm.getServiceName() -> String`
Copy link
Member

Choose a reason for hiding this comment

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

For Java, the API is an optional dependency and thus, we can't assume it's available even if the agent is installed. To set the service name in ECS logging, the agent has to instrument the ECS logger's constructors.

@felixbarny
Copy link
Member

felixbarny commented Jan 21, 2021

From the service.name spec:

When an APM agent is active, they should auto-configure it if not already set.

Just to clarify, this is not a required feature for the initial GA of ECS loggers. It's totally fine to add that as an additional feature afterwards.
It could be implemented in the context of #373, for example. Within that issue, agents would auto-configure the logging on behalf of the user, including setting the service.name.
From that, it's not far to instrument the construction of a formatter with the agent and set the service.name as detected by the agent.

@basepi
Copy link
Contributor

basepi commented Jan 21, 2021

"service.name" is not added if there is a log statement when there is no active transaction, then correct?

Correct. And yes, currently all of the injection happens on the agent side. Theoretically ecs-logging-python could access the transaction directly from the execution context but that's not how it's written currently. We'll probably want to clean it up once we start supporting log shipping in the agent directly.

@trentm
Copy link
Member Author

trentm commented Jan 28, 2021

As discussed on the APM Agents call today, I'm just going to close this as not worth the effort. While there was general murmuring that having documentation in this repo on API commonalities and guidance between APM agents, it was felt that it become more work and likely to get out of date. The alternative, when adding public API to an APM agent, is to poke around in the docs and code of other APM agents for prior art and to ask around in chat/email.

@trentm trentm closed this Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants