-
Notifications
You must be signed in to change notification settings - Fork 334
Service name auto discovery docs and polishing #2440
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
Changes from all commits
67da011
7895c65
7819fdb
d6e309b
7fb0c7d
8b63bf0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,8 +20,8 @@ | |
|
|
||
| import co.elastic.apm.agent.bci.TracerAwareInstrumentation; | ||
| import co.elastic.apm.agent.configuration.ServiceInfo; | ||
| import co.elastic.apm.agent.sdk.logging.Logger; | ||
| import co.elastic.apm.agent.sdk.logging.LoggerFactory; | ||
| import co.elastic.apm.agent.servlet.ServletServiceNameHelper; | ||
| import co.elastic.apm.agent.servlet.adapter.JavaxServletApiAdapter; | ||
| import net.bytebuddy.asm.Advice; | ||
| import net.bytebuddy.description.NamedElement; | ||
| import net.bytebuddy.description.method.MethodDescription; | ||
|
|
@@ -67,50 +67,47 @@ public String getAdviceClassName() { | |
|
|
||
| public static class SpringServiceNameAdvice { | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(SpringServiceNameAdvice.class); | ||
|
|
||
| @Advice.OnMethodExit(suppress = Throwable.class, inline = false) | ||
| public static void afterInitPropertySources(@Advice.This WebApplicationContext applicationContext) { | ||
| // avoid having two service names for a standalone jar | ||
| // one based on Implementation-Title and one based on spring.application.name | ||
| if (!ServiceInfo.autoDetected().isMultiServiceContainer()) { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some background: |
||
| return; | ||
| } | ||
| // This method will be called whenever the spring application context is refreshed which may be more than once | ||
| // | ||
| // For example, using Tomcat Servlet container, it's called twice with the first not having a ServletContext, | ||
| // while the second does, and later requests are initiated with the Servlet classloader and not the application | ||
| // classloader. | ||
| ClassLoader classLoader = applicationContext.getClassLoader(); | ||
|
|
||
| ServiceInfo fromServletContext = ServiceInfo.empty(); | ||
| ServletContext servletContext = applicationContext.getServletContext(); | ||
| if (servletContext != null) { | ||
| try { | ||
| ClassLoader servletClassloader = servletContext.getClassLoader(); | ||
| if (servletClassloader != null) { | ||
| classLoader = servletClassloader; | ||
| fromServletContext = ServletServiceNameHelper.detectServiceInfo(JavaxServletApiAdapter.get(), servletContext, servletClassloader); | ||
| } | ||
| } catch (UnsupportedOperationException e) { | ||
| // silently ignored | ||
| } | ||
| } | ||
|
|
||
| String appName = applicationContext.getEnvironment().getProperty("spring.application.name", ""); | ||
| ServiceInfo fromSpringApplicationNameProperty = ServiceInfo.of(applicationContext.getEnvironment().getProperty("spring.application.name", "")); | ||
| ServiceInfo fromApplicationContextApplicationName = ServiceInfo.of(removeLeadingSlash(applicationContext.getApplicationName())); | ||
|
|
||
| if (!appName.isEmpty()) { | ||
| if (logger.isDebugEnabled()) { | ||
| logger.debug("Setting service name `{}` to be used for class loader [{}], based on the value of " + | ||
| "the `spring.application.name` environment variable", appName, classLoader); | ||
| } | ||
| } else { | ||
| // fallback when application name isn't set through an environment property | ||
| appName = applicationContext.getApplicationName(); | ||
| // remove '/' (if any) from application name | ||
| if (appName.startsWith("/")) { | ||
| appName = appName.substring(1); | ||
| } | ||
| if (logger.isDebugEnabled()) { | ||
| logger.debug("``spring.application.name` environment variable is not set, falling back to using `{}` " + | ||
| "as service name for class loader [{}]", appName, classLoader); | ||
| } | ||
| } | ||
| tracer.overrideServiceInfoForClassLoader(classLoader, fromSpringApplicationNameProperty | ||
| .withFallback(fromServletContext) | ||
| .withFallback(fromApplicationContextApplicationName)); | ||
| } | ||
|
|
||
| tracer.overrideServiceInfoForClassLoader(classLoader, ServiceInfo.of(appName)); | ||
| private static String removeLeadingSlash(String appName) { | ||
| // remove '/' (if any) from application name | ||
| if (appName.startsWith("/")) { | ||
| appName = appName.substring(1); | ||
| } | ||
| return appName; | ||
| } | ||
| } | ||
| } | ||
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 think we could still honor the spring application name here in case the manifest entry is not set.
isMultiServiceContainer()returnfalseand thenServiceInfo.autoDetected()will match what was read in the manifestThere 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.
Clarified this through an offline discussion: we have another fallback here for the
ServiceInfo.autoDetected()that relies on the.jarname. Unfortunately when running in the IDE the test app is not packaged and thus not available.