Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closes #694 - Added option to expose counters as differences to influxDB exporter #695

Merged
merged 3 commits into from
May 4, 2020

Conversation

JonasKunz
Copy link
Member

@JonasKunz JonasKunz commented Apr 22, 2020

Closes #694


This change is Reviewable

@mariusoe mariusoe self-assigned this Apr 23, 2020
@JonasKunz JonasKunz force-pushed the influx-differences branch from 29fadb6 to 65cc077 Compare May 4, 2020 13:14
Copy link
Member

@mariusoe mariusoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 10 of 19 files at r1.
Reviewable status: 10 of 19 files reviewed, 10 unresolved discussions (waiting on @JonasKunz and @mariusoe)


components/inspectit-ocelot-eum-server/src/main/resources/application.yml, line 121 at r1 (raw file):

        # This difference will only be written if the counter has changed (=the difference is non-zero).
        # This can greatly reduce the total data written to influx and makes writing queries easier.
        counters-as-differences: true

I think we have to discuss this.
I'm not sure if we should set this to true. This way, we break existing things because the exporting values are suddenly different after upgrading.
On the other hand, users may not change this behaviour at all.. Mabye we should keep this false by default for some versions..?


inspectit-ocelot-config/src/main/resources/rocks/inspectit/ocelot/config/default/exporters.yml, line 47 at r1 (raw file):

        # This difference will only be written if the counter has changed (=the difference is non-zero).
        # This can greatly reduce the total data written to influx and makes writing queries easier.
        counters-as-differences: true

See comment in the EUM configuration regarding this field.


inspectit-ocelot-documentation/docs/breaking-changes/1.3.md, line 22 at r1 (raw file):

InfluxDB exporter

..InfluxDB metrics exporter..


inspectit-ocelot-documentation/docs/breaking-changes/1.3.md, line 22 at r1 (raw file):

and sum metrics, which is now the default behaviour.

let's discuss this


inspectit-ocelot-documentation/docs/breaking-changes/1.3.md, line 23 at r1 (raw file):

influxDB

Let's write it as InfluxDB


inspectit-ocelot-documentation/docs/breaking-changes/1.3.md, line 23 at r1 (raw file):

your queries will need to be reworked.

...your existing queries have to be reworked.


inspectit-ocelot-documentation/docs/breaking-changes/1.3.md, line 24 at r1 (raw file):

 old 

previous


inspectit-ocelot-documentation/docs/metrics/metric-exporters.md, line 59 at r1 (raw file):

When querying the data, usually the absolute value of such counters is irrelevant.
Instead all that matters is how much the counter has increased within a certain time interval.

Usually, the absolute value of such counters is irrelevant when querying the data, instead you want to have the increase of it over a certain period of time..


inspectit-ocelot-documentation/docs/metrics/metric-exporters.md, line 61 at r1 (raw file):

Instead of writing the absolute value of each counter to InfluxDB, only the increase since the last export is exported.

Instead of writing the absolute value of each counter into the InfluxDB, only the increase since the last export will be written.


inspectit-ocelot-documentation/docs/metrics/metric-exporters.md, line 63 at r1 (raw file):

to influx in case

...into the InfluxDB, especially if the metrics are quite constant and won't change much.

Copy link
Member

@mariusoe mariusoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 9 of 19 files at r1, 3 of 3 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@mariusoe mariusoe merged commit a0fb753 into inspectIT:master May 4, 2020
@JonasKunz JonasKunz deleted the influx-differences branch May 26, 2020 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants