add java attacher config #5483
Merged
add java attacher config #5483
Conversation
This file contains 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
💔 Build Failed
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪Steps errors
Expand to view the steps failures
|
re-add it at some point when we want to display the version
simitt
reviewed
Jun 17, 2021
simitt
approved these changes
Jun 18, 2021
There was a problem hiding this comment.
LGTM! Let's get @felixbarny involved to take a look at the config options.
beater/config/java_attacher.go
Outdated
| // attacher jarfile. | ||
| type JavaAttacherConfig struct { | ||
| Enabled bool `config:"enabled"` | ||
| Continuous bool `config:"continuous"` |
There was a problem hiding this comment.
I'd rather expect that the apm-server would always set continuous when starting the attacher; but ultimately leave that to @felixbarny .
There was a problem hiding this comment.
👍 will see what he says
There was a problem hiding this comment.
+1 on always implicitly using the continuous mode.
felixbarny
reviewed
Jun 18, 2021
beater/config/java_attacher.go
Outdated
| // attacher jarfile. | ||
| type JavaAttacherConfig struct { | ||
| Enabled bool `config:"enabled"` | ||
| Continuous bool `config:"continuous"` |
There was a problem hiding this comment.
+1 on always implicitly using the continuous mode.
felixbarny
requested changes
Jun 18, 2021
|
For config:
- "service_name=my-cool-service"
- "server_url=http://localhost:8200"Could we do something like this instead? config:
service_name: my-cool-service
server_url: http://localhost:8200 |
felixbarny
approved these changes
Jun 22, 2021
beater/config/java_attacher.go
Outdated
| IncludeAll bool `config:"include_all"` | ||
| IncludePID []string `config:"include_pid"` | ||
|
|
||
| IncludeMain []string `config:"include_main"` |
There was a problem hiding this comment.
A note for the implementation: All the values will have to be escaped when passing them to the attacher to avoid injection attack vectors.
Example:
- include_main: "foo; rm -rf /"
should not lead to the hard drive being purged
|
jenkins run the tests |
mergify bot
pushed a commit
that referenced
this pull request
Jun 24, 2021
* add java attacher config (cherry picked from commit 92f0452)
stuartnelson3
added a commit
that referenced
this pull request
Jun 28, 2021
* add java attacher config (cherry picked from commit 92f0452) Co-authored-by: stuart nelson <[email protected]>
|
I'm removing this from the 7.14 test plan, as the feature can't be used yet. Let's test the config as part of the feature when we deliver it. |
mergify bot
pushed a commit
that referenced
this pull request
Jul 6, 2021
* add java attacher config (cherry picked from commit 92f0452)
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.
Motivation/summary
Adding basic include/exclude and config flags.
Based on this comment I pulled out version as something to be static for a given build of apm-server, which we update when we bundle with a new version of the attacher.
closes #4829
Checklist
For functional changes, consider:
How to test these changes
Update the
apm-server.ymlto include java attacher config fields, and verify they're parsed.eg.
Related issues