Add node-observ-lib #25

v-zhuravlev · 2023-07-24T13:33:31Z

Similar to windows-observ-lib, node-observ-lib is created, using many panels from commonlib:. It also refactors everything from node-mixin by using grafonnet (v10 schema).

Node observ lib would allow:

Easily read and maintain dashboards as code
Easily modify any dashboard, panel, target, variable... before rendering final result
Change instanceLabels,groupLabels from default job,instance to use dashboards in environments with extra or custom labels
Instantiate node dashboards more than once in a single environment (Uid and filteringSelector are used to avoid conflicts).

Due to current incompatibility with old mixin, lib is created in a separate folder.

What is not implemented (yet?):

MacOS dashboard (will add in next iteration)
USE dashboards (node-rsrc-use.json, node-cluster-rsrc-use.json,node-multicluster-rsrc-use.json) (decide if needs re implementing or just drop)

What is recreated/moved from old node-mixin:

All dashboards, except mentioned above
All alerts
All recording rules

Added:

New loki annotations (only added if enableLokiLogs:true)
- Service failed
- Critical system event
- Session opened
- Session closed
Title case is used across all recreated panels and dashboard names

rgeyer

LGTM

Couple nits/comments.

rgeyer · 2023-11-09T20:27:02Z

docs/node-observ-lib/annotations.libsonnet

+          {
+          hide: true,
+          iconColor: 'light-purple',
+        },


Couldn't you use commonlib.annotations.fatal.new instead? These values are implied there 🤔

Technically, it is not fatal event. So maybe it is best to change color instead :)

rgeyer · 2023-11-09T20:39:32Z

docs/node-observ-lib/main.libsonnet

+  ): {
+
+    local this = self,
+    config: {


Did we decide and document somewhere that config should go in the main definition of the mixin?

We have an existing pattern of looking for an external config.libsonnet file to change configurable options, so this could be confusing.

Nope, there was no decision if config should be inline or in separate file. I'll see what I can do.

Decided to refactor it. New() is now parameterless, and all config is moved to config.libsonnet file. Some sane defaults are provided for all params required in modular libs. And any of those defaults can be modified by calling it like this:

local nodelib = import './main.libsonnet'; local linux = nodelib.new() + nodelib.withConfigMixin({ filteringSelector: 'job=~".*node.*"', groupLabels: ['job'], instanceLabels: ['instance'], dashboardNamePrefix: 'Node exporter / ', dashboardTags: ['node-exporter-mixin'], uid: 'node', });

This looks great to me! Makes the config a bit more modular and clear where the changeable parameters are.

I like the withConfigMixin function too, allowing the user to easily supply their own config overrides.

This makes sense to me, I'd be curious if the rest of the team has any feedback. @Dasomeone @gaantunes ?

Sorry for missing this ping (and the PR, will review that as well)

I really like the solution you've come up with here, with config separate by default but a function to supply in-line should it be desired. Quite happy to go forward with what the two of you have already done :D

Dasomeone · 2023-11-14T11:19:36Z

Just a comment here, based on the description:

Due to current incompatibility with old mixin, lib is created in a separate folder.

Is there a list of the incompatibilities? Is it just the two listed for macos and USE dashboards? Are we tracking that in the issue or separate issue?

gaantunes

Looks amazing @v-zhuravlev, very well organized and easy to customize.
I also love that even though it is a lot of jsonnet code, it is easy to understand what is happening. Our previous approach of putting everything in just a few files was hard to grok.

Dasomeone

Couple very minor nitpicks but generally looks fantastic, great work Vitaly!!

Dasomeone · 2023-11-14T12:26:59Z

docs/node-observ-lib/README.md

Minor nitpicks throughout: I don't mind shortening observability to observ for the folder name, but it feels pretty awkward everywhere else

Good one, updated readme

Dasomeone · 2023-11-14T12:28:11Z

docs/node-observ-lib/README.md

+
+local linux =
+  nodelib.new()
+  + nodelib.withConfigMixin({


Maybe we should add a little explanation/example with separate configuration?

docs/node-observ-lib/alerts.libsonnet

v-zhuravlev · 2023-11-14T13:51:20Z

Just a comment here, based on the description:

Due to current incompatibility with old mixin, lib is created in a separate folder.

Is there a list of the incompatibilities? Is it just the two listed for macos and USE dashboards? Are we tracking that in the issue or separate issue?

yes, those are major ones. Also some config params are dropped, like clusterLabel...

* Add node-observ-lib * Remove trends support (not in 10.0 schema) * Make filteringSelector for logs dashboard configurable * Temp change dependency (until PR is merged for commonlib) * Refactor config * Update jsonnetfile.json * Update README * Add separate loki example * Add sep file example Signed-off-by: Vitaly Zhuravlev <v-zhuravlev@users.noreply.github.com>

Add node-observ-lib

8649f29

v-zhuravlev force-pushed the vzhuravlev/common-lib-example branch from 7577cf2 to 8649f29 Compare November 9, 2023 12:09

v-zhuravlev marked this pull request as ready for review November 9, 2023 12:20

v-zhuravlev changed the title ~~Example usage of jsonnet-libs/common-lib/common~~ Add node-observ-lib Nov 9, 2023

v-zhuravlev added 3 commits November 9, 2023 15:15

Remove trends support (not in 10.0 schema)

f5802af

Make filteringSelector for logs dashboard configurable

db019c5

Temp change dependency (until PR is merged for commonlib)

79b4153

rgeyer approved these changes Nov 9, 2023

View reviewed changes

v-zhuravlev added 2 commits November 13, 2023 17:24

Refactor config

784cf59

Update jsonnetfile.json

d9f8ea2

gaantunes approved these changes Nov 14, 2023

View reviewed changes

Dasomeone approved these changes Nov 14, 2023

View reviewed changes

v-zhuravlev added 3 commits November 14, 2023 13:52

Update README

016bfac

Add separate loki example

0e78ebf

Add sep file example

21c8272

v-zhuravlev merged commit 836773a into master Nov 14, 2023
2 checks passed

v-zhuravlev deleted the vzhuravlev/common-lib-example branch November 14, 2023 14:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add node-observ-lib #25

Add node-observ-lib #25

v-zhuravlev commented Jul 24, 2023 •

edited

Loading

rgeyer left a comment

rgeyer Nov 9, 2023

v-zhuravlev Nov 11, 2023

rgeyer Nov 9, 2023

v-zhuravlev Nov 10, 2023

v-zhuravlev Nov 13, 2023 •

edited

Loading

rgeyer Nov 13, 2023

Dasomeone Nov 14, 2023

Dasomeone commented Nov 14, 2023

gaantunes left a comment

Dasomeone left a comment

Dasomeone Nov 14, 2023

v-zhuravlev Nov 14, 2023

Dasomeone Nov 14, 2023

v-zhuravlev Nov 14, 2023

v-zhuravlev commented Nov 14, 2023

Add node-observ-lib #25

Add node-observ-lib #25

Conversation

v-zhuravlev commented Jul 24, 2023 • edited Loading

rgeyer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

v-zhuravlev Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dasomeone commented Nov 14, 2023

gaantunes left a comment

Choose a reason for hiding this comment

Dasomeone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

v-zhuravlev commented Nov 14, 2023

v-zhuravlev commented Jul 24, 2023 •

edited

Loading

v-zhuravlev Nov 13, 2023 •

edited

Loading