Skip to content

Conversation

@felixbarny
Copy link
Member

@felixbarny felixbarny commented Sep 27, 2019

closes #864

Checklist

@codecov-io
Copy link

codecov-io commented Sep 27, 2019

Codecov Report

Merging #865 into master will decrease coverage by 0.6%.
The diff coverage is 56.25%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #865      +/-   ##
============================================
- Coverage     64.83%   64.22%   -0.61%     
- Complexity       84       85       +1     
============================================
  Files           230      230              
  Lines          9449     9358      -91     
  Branches       1245     1221      -24     
============================================
- Hits           6126     6010     -116     
- Misses         2945     2963      +18     
- Partials        378      385       +7
Impacted Files Coverage Δ Complexity Δ
...nfiguration/AgentArgumentsConfigurationSource.java 92.3% <ø> (ø) 0 <0> (ø) ⬇️
...ration/source/PropertyFileConfigurationSource.java 30.26% <ø> (ø) 0 <0> (ø) ⬇️
...java/co/elastic/apm/attach/ElasticApmAttacher.java 20.54% <44.44%> (+8.26%) 4 <1> (+1) ⬆️
...lastic/apm/agent/impl/ElasticApmTracerBuilder.java 79.01% <71.42%> (-0.4%) 0 <0> (ø)
.../apm/agent/report/serialize/DslJsonSerializer.java 86.08% <0%> (-3.63%) 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d54da16...69b69ac. Read the comment docs.

@hcf
Copy link

hcf commented Sep 30, 2019

Could it be possible to use system properties instead as suggested in the temporary fix for my problem?

configuration.forEach((key, value) -> System.setProperty(String.format("elastic.apm.%s", key), value));

Not sure what is best for concurrency, conflicts, etc.

@felixbarny
Copy link
Member Author

That wouldn't work with the external attachment (https://www.elastic.co/guide/en/apm/agent/java/current/setup-attach-cli.html). Do you see an issue with the proposed approach?

@hcf
Copy link

hcf commented Sep 30, 2019

The only issue I can think about is what happens if we start multiple JVM on the same machine at the same time?

@felixbarny
Copy link
Member Author

Then each JVM will get their own temporary file (with a random name) which gets deleted after the attachment for that JVM is completed.

@felixbarny felixbarny merged commit 38e92dc into elastic:master Sep 30, 2019
@felixbarny felixbarny deleted the agent-args-temp-properties branch September 30, 2019 10:10
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.

Segmentation fault when attaching apm-agent-java to adopt jdk 11

3 participants