feat #1124: Create agent command for fetching details about an agent #1473
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1473 +/- ##
============================================
+ Coverage 79.20% 79.26% +0.06%
- Complexity 2250 2255 +5
============================================
Files 230 231 +1
Lines 7292 7305 +13
Branches 867 867
============================================
+ Hits 5775 5790 +15
+ Misses 1159 1157 -2
Partials 358 358
|
There was a problem hiding this comment.
Reviewed 9 of 9 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @mariusoe and @Napfschnecke)
components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentcommunication/handlers/impl/EnvironmentCommandHandler.java line 1 at r1 (raw file):
package rocks.inspectit.ocelot.agentcommunication.handlers.impl;
Please add some tests for this class.
components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentcommunication/handlers/impl/EnvironmentCommandHandler.java line 76 at r1 (raw file):
* Takes an instance of {@link CommandResponse} as well as an instance of {@link DeferredResult}. Sets the * {@link ResponseEntity} of the {@link DeferredResult} to the status OK. In this handler the given response is * ignored since the response itself indicates that the agent is alive.
This description here is not correct. The response is not ignored, it is used further to build the file containing the general information about the agent.
inspectit-ocelot-config/src/main/java/rocks/inspectit/ocelot/commons/models/command/impl/EnvironmentCommand.java line 16 at r1 (raw file):
/** * Represents an Environment-Command. Environment commands are used to receive the details of a certain agent.
You could add here a short description on what these details are.
inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/command/handler/impl/EnvironmentCommandExecutor.java line 30 at r1 (raw file):
/** * Executes the given {@link Command}. Throws an {@link IllegalArgumentException} if the given command is either null
You could add a short description how the command is executes (i.E. it Populates an instance of EnvironmentCommand.EnvironmentDetail with the respective values from the agent).
inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/command/handler/impl/EnvironmentCommandExecutorTest.java line 82 at r1 (raw file):
@DirtiesContext public class Execute {
Could you also add a test for the case that canExecute returns "false" in the execution method? I think it's a low hanging fruit and we will achieve a 100% test coverage of the class.
sbraitsch
force-pushed
the
feat/feature-1124
branch
from
2e72297
to
66274ef
Compare
There was a problem hiding this comment.
Reviewable status: 5 of 10 files reviewed, 5 unresolved discussions (waiting on @MariusBrill and @mariusoe)
components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentcommunication/handlers/impl/EnvironmentCommandHandler.java line 1 at r1 (raw file):
Previously, MariusBrill (Marius Brill) wrote…
Please add some tests for this class.
Done.
components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentcommunication/handlers/impl/EnvironmentCommandHandler.java line 76 at r1 (raw file):
Previously, MariusBrill (Marius Brill) wrote…
This description here is not correct. The response is not ignored, it is used further to build the file containing the general information about the agent.
Done.
inspectit-ocelot-config/src/main/java/rocks/inspectit/ocelot/commons/models/command/impl/EnvironmentCommand.java line 16 at r1 (raw file):
Previously, MariusBrill (Marius Brill) wrote…
You could add here a short description on what these details are.
Done.
inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/command/handler/impl/EnvironmentCommandExecutor.java line 30 at r1 (raw file):
Previously, MariusBrill (Marius Brill) wrote…
You could add a short description how the command is executes (i.E. it Populates an instance of EnvironmentCommand.EnvironmentDetail with the respective values from the agent).
Done.
inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/command/handler/impl/EnvironmentCommandExecutorTest.java line 82 at r1 (raw file):
Previously, MariusBrill (Marius Brill) wrote…
Could you also add a test for the case that canExecute returns "false" in the execution method? I think it's a low hanging fruit and we will achieve a 100% test coverage of the class.
Done.
There was a problem hiding this comment.
Reviewed 5 of 5 files at r2.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @mariusoe and @Napfschnecke)
components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentcommunication/handlers/impl/EnvironmentCommandHandler.java line 76 at r1 (raw file):
Previously, Napfschnecke (sbr) wrote…
Done.
I think this description is still not quite correct. How about:
"Takes an instance of {@link CommandResponse} as well as an instance of {@link DeferredResult}. Sets the {@link ResponseEntity} of the {@link DeferredResult} according to the Environment Information received from the respective Agent."
sbraitsch
force-pushed
the
feat/feature-1124
branch
from
66274ef
to
45fda15
Compare
sbraitsch
force-pushed
the
feat/feature-1124
branch
from
45fda15
to
a0ca0dc
Compare
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @mariusoe)
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @Napfschnecke)
Closes #1124
Added new command endpoint /command/environment?agent-id=xxx to retrieve info about environment variables, jvm properties and the arguments passed to the jvm on startup.
This change is