Closes #980 - Extending the alerting API to reload kapacitor tasks due to changed alerting rules

[Patrick-Eichhorn]

Closes #980

---

This change is 

[mariusoe]

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Patrick-Eichhorn)

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/KapacitorTaskController.java, line 61 at r1 (raw file):

    public Task updateTask(@PathVariable @ApiParam("The id of the task to update") String taskId, @RequestBody Task task) {
        ObjectNode response = kapacitor().patchForObject("/kapacitor/v1/tasks/{taskId}", task.toKapacitorRequest(), ObjectNode.class, taskId);
        ObjectNode responseReload = kapacitor().patchForObject("/kapacitor/v1/templates/{templateID}", task.toKapacitorRequestTemplateUpdate(), ObjectNode.class, task

You don't have to store this in a variable because you don't need it.

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/KapacitorTaskController.java, line 61 at r1 (raw file):

    public Task updateTask(@PathVariable @ApiParam("The id of the task to update") String taskId, @RequestBody Task task) {
        ObjectNode response = kapacitor().patchForObject("/kapacitor/v1/tasks/{taskId}", task.toKapacitorRequest(), ObjectNode.class, taskId);
        ObjectNode responseReload = kapacitor().patchForObject("/kapacitor/v1/templates/{templateID}", task.toKapacitorRequestTemplateUpdate(), ObjectNode.class, task

Can you move this in a separate method? Imo it is more clearly what's happening and also why:

private void triggerTaskReload(Task task) {
    String taskTemplateId = task.getTemplate();
    kapacitor().patchForObject("/kapacitor/v1/templates/{templateID}", task.toKapacitorRequestTemplateUpdate(), ObjectNode.class, taskTemplateId);
}

then you just need to call triggerTaskReload(task); when a task should be reloaded

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/model/Task.java, line 114 at r1 (raw file):

     * @return a JSON-Object which can be used for updating the template in PATCH request to kapacitor
     */
    public ObjectNode toKapacitorRequestTemplateUpdate() {

Do we need tests for this?

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/model/Task.java, line 115 at r1 (raw file):

     */
    public ObjectNode toKapacitorRequestTemplateUpdate() {
        ObjectMapper mapper = new ObjectMapper();

You don't need to create a new object mapper every time. It's recommended to just create one and reuse it.

You can convert it to a final and static field and initialize it once:

private static final ObjectMapper mapper = new ObjectMapper();

Then in your method - same in the toKapacitorRequest - you can just use it.

[Patrick-Eichhorn]

Reviewable status: 1 of 3 files reviewed, 4 unresolved discussions (waiting on @mariusoe)

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/KapacitorTaskController.java, line 61 at r1 (raw file):

Previously, mariusoe (Marius Oehler) wrote…

You don't have to store this in a variable because you don't need it.

Done.

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/KapacitorTaskController.java, line 61 at r1 (raw file):

Previously, mariusoe (Marius Oehler) wrote…

Can you move this in a separate method? Imo it is more clearly what's happening and also why:

private void triggerTaskReload(Task task) {
    String taskTemplateId = task.getTemplate();
    kapacitor().patchForObject("/kapacitor/v1/templates/{templateID}", task.toKapacitorRequestTemplateUpdate(), ObjectNode.class, taskTemplateId);
}

then you just need to call triggerTaskReload(task); when a task should be reloaded

Done.

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/model/Task.java, line 114 at r1 (raw file):

Previously, mariusoe (Marius Oehler) wrote…

Do we need tests for this?

Done.

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/model/Task.java, line 115 at r1 (raw file):

Previously, mariusoe (Marius Oehler) wrote…

You don't need to create a new object mapper every time. It's recommended to just create one and reuse it.

You can convert it to a final and static field and initialize it once:

private static final ObjectMapper mapper = new ObjectMapper();

Then in your method - same in the toKapacitorRequest - you can just use it.

Done.

[mariusoe]

Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Patrick-Eichhorn)

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/KapacitorTaskController.java, line 73 at r2 (raw file):

    }

    private void triggerTaskReload(Task task) {

Please add a small comment, also why we do an update of the template for this.

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/model/Task.java, line 63 at r2 (raw file):

    List<TemplateVariable> vars;

    private static final ObjectMapper mapper = new ObjectMapper();

Please move static variables to the top of the class - before member fields.

[Patrick-Eichhorn]

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @mariusoe)

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/KapacitorTaskController.java, line 73 at r2 (raw file):

Previously, mariusoe (Marius Oehler) wrote…

Please add a small comment, also why we do an update of the template for this.

Done.

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/model/Task.java, line 63 at r2 (raw file):

Previously, mariusoe (Marius Oehler) wrote…

Please move static variables to the top of the class - before member fields.

Done.

[mariusoe]

Reviewed 2 of 2 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Patrick-Eichhorn)

---

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/alert/kapacitor/KapacitorTaskController.java, line 73 at r2 (raw file):

Previously, Patrick-Eichhorn wrote…

Done.

I thought more like why we are patching the template for this. Maybe we can write a small comment that a task cannot be reloaded and just doing patch request against the template api - without changing it - triggers kapacitor to reload the previously changed task.

[Patrick-Eichhorn]

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @mariusoe)

[mariusoe]

Reviewed 1 of 1 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved

[mariusoe]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Patrick-Eichhorn)