Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
On Windows Azure Functions, the compat agent now gives each function app its own trace and DogStatsD named pipes, as the .NET compatibility layer has done since v1.5.0. In
premainit hands the chosen names todd-java-agentas thedd.trace.pipe.nameanddd.dogstatsd.pipe.namesystem properties. The binary gets the same names asDD_APM_WINDOWS_PIPE_NAMEandDD_DOGSTATSD_WINDOWS_PIPE_NAME.NamedPipeNamesfollows the .NETCompatibilityLayerorder for each pipe:DD_APM_WINDOWS_PIPE_NAME/DD_DOGSTATSD_WINDOWS_PIPE_NAME, used as written.-Ddd.trace.pipe.name/-Ddd.dogstatsd.pipe.name, elseDD_TRACE_PIPE_NAME/DD_DOGSTATSD_PIPE_NAME.dd_trace_<guid>/dd_dogstatsd_<guid>.It logs a warning when a customer-set name differs from the
*_WINDOWS_PIPE_NAMEvalue, and when a name is longer than 256 characters.Pipes turn on only when the
dd-java-agentin the JVM is new enough.TracerVersionGatefinds the tracer among the-javaagentoptions by its manifestPremain-Class(datadog.trace.bootstrap.AgentPreCheck), so a renamed jar still counts. It compares the major.minor.patch of the tracer'sImplementation-Versionand ignores suffixes such as-SNAPSHOT. If the tracer is older, missing, or unreadable, the agent logs why at INFO and doesn't add pipe settings anywhere. Inherited environment variables stay as they are. Those apps keep today's TCP 8126 and UDP 8125 behavior, and pipe settings a customer configured by hand behave as they do today.Motivation
SVLS-9467 / SVLS-8245. Java function apps that share an Elastic Premium or Dedicated plan also share instances. Their binaries collide on one pipe name or port. Shared TCP isn't a fallback on Windows, because the Azure sandbox only allows connections to
localhostlisteners in the function's own app. A tracer whose binary lost the race for 8126 getsPermission denied. Per-function pipes fixed this for .NET (datadog-serverless-compat-dotnet#20) and Node (datadog-serverless-compat-js#46). Java uses system properties instead of environment variables because a JVM can't change its own environment after it starts, and dd-trace-java'sConfigalready readsdd.*.pipe.nameproperties ahead of environment variables.The gate exists because no released dd-trace-java can use a pipe. Trace upload hangs forever on the tracer's shared pipe socket, and the tracer's StatsD client throws a
NullPointerExceptionwhenever it has a pipe name. The fixes for both are in DataDog/dd-trace-java#12898.Additional Notes
This can't merge until two releases exist:
datadog-serverless-compatbinary release with serverless-components#181. That change keeps the binary's DogStatsD listening on UDP 8125 next to the pipe. Without it, customerjava-dogstatsd-clientcode that sends tolocalhost:8125loses its metrics.TracerVersionGate.MINIMUM_TRACER_VERSIONis1.68.0, a placeholder for that release. It's the next unreleased version on dd-trace-java master: master's latest tag is v1.67.0 (v1.67.1 sits on a release branch), and dd-trace-java's version plugin bumps the minor for untagged commits, so master builds report1.68.0-SNAPSHOT. The constant moves to the actual release before this merges. If the fixes miss 1.68.0, a 1.68.x tracer without them would pass the gate as written.Design choices and known gaps:
DD_APM_WINDOWS_PIPE_NAME, the variable the binary reads. .NET'sCompatibilityLayerreadsDD_TRACE_WINDOWS_PIPE_NAMEfor that step.-Ddd.trace.pipe.nameandDD_TRACE_PIPE_NAME, the agent uses the JVM option, matching dd-trace-java's own config order. DogStatsD names follow the same order.-javaagentoptions fromRuntimeMXBean.getInputArguments(), and only on Windows Azure Functions. dd-trace-java avoidsManagementFactoryin its own premain because on some JDKs it loadsjava.util.logging.LogManagerearly, which breaks custom log managers. I don't know yet whether any Azure Functions Java app depends on a custom JVM-wide log manager, so I didn't copy dd-trace-java's reflection workaround.dd-java-agentdoesn't get pipes. Its custom metrics stay on shared UDP 8125, as in .NET.java-dogstatsd-clientdefaults ignoredd.dogstatsd.pipe.nameuntil the client change for SVLS-8246 is in a client release.Describe how to test/QA your changes
mvn testruns 53 tests, 37 of them new:NamedPipeNamesTestcovers the name order, JVM options beating environment variables, empty values, generated names, and both warnings.TracerVersionGateTestchecks that the minimum, later versions, and dev builds of the minimum (1.68.0-SNAPSHOT,1.68.0-105-gefa6bd64) pass. Older, missing, non-jar, unversioned, and unparseable tracers fail. One test renames the tracer jar to prove the gate matches onPremain-Class. The tests write real jars with manifests to a temp directory.I also ran the gate in a JVM started with this agent and a real
dd-java-agent-1.59.0.jaras-javaagentoptions. It skipped the compat jar, found the tracer, and reported 1.59.0 as older than 1.68.0. Thepremainwiring has no unit test. The planned end-to-end run on shared EP2 and P1V3 plans in serverless-compat-self-monitoring covers it, with dev builds of the binary, this agent, and dd-trace-java. That run hasn't happened yet.