Skip to content
This repository has been archived by the owner on Mar 4, 2024. It is now read-only.

EVEREST-507 Report metrics #252

Merged
merged 6 commits into from
Oct 20, 2023
Merged

Conversation

oksana-grishchenko
Copy link
Contributor

@oksana-grishchenko oksana-grishchenko commented Oct 19, 2023

EVEREST-507 Powered by Pull Request Badge

Collect and report metrics

Problem:
EVEREST-507

Report the db engines metrics - how many db clusters of each type are running.

A step forward multi k8s support - generate own Everest ID instead of using the k8s cluster ID.

The telemetry request example

CHECKLIST

Jira

  • Is the Jira ticket created and referenced properly?

Tests

  • Is an Integration test/test case added for the new feature/change?
  • Are unit tests added where appropriate?

@oksana-grishchenko oksana-grishchenko marked this pull request as ready for review October 19, 2023 08:46
@oksana-grishchenko oksana-grishchenko requested a review from a user October 19, 2023 08:46
Copy link
Collaborator

@recharte recharte left a comment

Choose a reason for hiding this comment

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

Overall the code looks good but I'm not entirely sure that having a table called settings with a single ID column is the best long term solution.
Maybe we should go with a key value approach that will not only cover our current use-case of storing the unique everest instance ID but will allow us to expand it to any other settings that we need to store like default values for a DB engine configuration or whatever.
What do you think about this?

model/settings.go Outdated Show resolved Hide resolved
api/everest.go Outdated Show resolved Hide resolved
api/everest.go Outdated Show resolved Hide resolved
api/telemetry.go Outdated Show resolved Hide resolved
@oksana-grishchenko
Copy link
Contributor Author

Maybe we should go with a key value approach that will not only cover our current use-case of storing the unique everest instance ID but will allow us to expand it to any other settings that we need to store like default values for a DB engine configuration or whatever.

@recharte thanks for the suggestion, the key-value approach sounds great. Updated, PTAL

Copy link
Collaborator

@recharte recharte left a comment

Choose a reason for hiding this comment

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

Looks great 💪

@oksana-grishchenko oksana-grishchenko requested a review from a user October 20, 2023 07:56
api/telemetry.go Outdated Show resolved Hide resolved
model/settings.go Outdated Show resolved Hide resolved
@oksana-grishchenko oksana-grishchenko requested a review from a user October 20, 2023 09:19
@oksana-grishchenko oksana-grishchenko merged commit ad9c7f7 into main Oct 20, 2023
@oksana-grishchenko oksana-grishchenko deleted the EVEREST-507-send-metrics branch October 20, 2023 10:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants