TrueCloudLab/frostfs-node
21
1
Fork 27
Code Issues 108 Pull requests 3 Projects Releases 1 Activity Actions

metrics: Export log and morph with script #718

Merged
fyrchik merged 4 commits from acid-ant/frostfs-node:bugfix/680-export-metrics into master 2024-09-04 19:51:03 +00:00
Conversation 7 Commits 4 Files changed 12 +78 -41
acid-ant commented 2023-10-04 07:10:39 +00:00
Member
Copy link

Close #680

Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com

Close #680 Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
acid-ant requested review from storage-core-committers 2023-10-04 07:12:18 +00:00
acid-ant requested review from storage-core-developers 2023-10-04 07:12:24 +00:00
dstepanov-yadro approved these changes 2023-10-04 08:40:11 +00:00
fyrchik approved these changes 2023-10-05 13:13:47 +00:00
Makefile Outdated
@ -71,2 +71,4 @@
go mod tidy -v && echo OK
# Build export-metrics
export-metrics: dep
fyrchik commented 2023-10-05 13:13:31 +00:00
Owner
Copy link

Why did you decide to make a separate target?

Why did you decide to make a separate target?
acid-ant commented 2023-10-05 13:44:20 +00:00
Author
Member
Copy link

Just to start a discussion, was it really necessary or not. Think it is a good option to have an ability to build this script. Have no understanding by whom the script is used.

Just to start a discussion, was it really necessary or not. Think it is a good option to have an ability to build this script. Have no understanding by whom the script is used.
Makefile Outdated
@ -73,0 +75,4 @@
@printf "⇒ Build export-metrics\n"
CGO_ENABLED=0 \
go build -v -trimpath \
-ldflags "-X $(REPO)/misc.Version=$(VERSION)" \
fyrchik commented 2023-10-05 13:12:00 +00:00
Owner
Copy link

Can we indent continuation of go build one level to the right? It's a bit easier to read IMO

Can we indent continuation of `go build` one level to the right? It's a bit easier to read IMO
fyrchik commented 2023-10-05 13:13:44 +00:00
Owner
Copy link

Also, do we need VERSION here?

Also, do we need `VERSION` here?
fyrchik commented 2023-10-05 13:20:34 +00:00
Owner
Copy link

It may be easy to forget to add metrics defined this way: have you considered adding them to NodeMetrics?

It may be easy to forget to add metrics defined this way: have you considered adding them to `NodeMetrics`?
acid-ant force-pushed bugfix/680-export-metrics from 4553f09c6b to d952d8aedf 2023-10-05 14:34:19 +00:00 Compare
acid-ant commented 2023-10-05 14:36:35 +00:00
Author
Member
Copy link

It may be easy to forget to add metrics defined this way: have you considered adding them to NodeMetrics?

Let's initialize morph metrics together with node. Don't know why @ale64bit init it separately.

> It may be easy to forget to add metrics defined this way: have you considered adding them to `NodeMetrics`? Let's initialize morph metrics together with node. Don't know why @ale64bit init it separately.
fyrchik reviewed 2023-10-05 14:37:13 +00:00
scripts/export-metrics/main.go Outdated
@ -27,9 +28,11 @@ func main() {
switch {
case *node != "":
_ = local_metrics.NewNodeMetrics()
_ = logger.NewLogMetrics("frostfs_node")
fyrchik commented 2023-10-05 14:37:13 +00:00
Owner
Copy link

Can we move this to a local_metrics too?

Can we move this to a `local_metrics` too?
fyrchik marked this conversation as resolved
fyrchik reviewed 2023-10-05 14:37:15 +00:00
scripts/export-metrics/main.go Outdated
@ -27,9 +28,11 @@ func main() {
switch {
case *node != "":
_ = local_metrics.NewNodeMetrics()
_ = logger.NewLogMetrics("frostfs_node")
fyrchik commented 2023-10-05 14:37:15 +00:00
Owner
Copy link

Can we move this to a local_metrics too?

Can we move this to a `local_metrics` too?
fyrchik commented 2023-10-05 14:37:15 +00:00
Owner
Copy link

Can we move this to a local_metrics too?

Can we move this to a `local_metrics` too?
fyrchik commented 2023-10-05 14:38:10 +00:00
Owner
Copy link

I love responsive UIs!

I love responsive UIs!
acid-ant commented 2023-10-05 14:51:35 +00:00
Author
Member
Copy link

Me too!

Me too!
fyrchik marked this conversation as resolved
fyrchik approved these changes 2023-10-05 14:37:53 +00:00
fyrchik left a comment
Owner
Copy link

Also, is everything OK with gRPC metrics? They must've missed last time I checked (duration of the request etc.)

Also, is everything OK with gRPC metrics? They must've missed last time I checked (duration of the request etc.)
scripts/export-metrics/main.go Outdated
@ -27,9 +28,11 @@ func main() {
switch {
case *node != "":
_ = local_metrics.NewNodeMetrics()
_ = logger.NewLogMetrics("frostfs_node")
fyrchik commented 2023-10-05 14:37:17 +00:00
Owner
Copy link

Can we move this to a local_metrics too?

Can we move this to a `local_metrics` too?
acid-ant commented 2023-10-08 09:33:44 +00:00
Author
Member
Copy link

Done, please review.

Done, please review.
acid-ant changed title from metrics: Export log and morph with script to WIP: metrics: Export log and morph with script 2023-10-05 14:47:30 +00:00
acid-ant force-pushed bugfix/680-export-metrics from d952d8aedf to fcd328c84c 2023-10-05 14:49:33 +00:00 Compare
acid-ant commented 2023-10-05 14:52:26 +00:00
Author
Member
Copy link

Move it to DRAFT for a while to solve an issue with commits.

Move it to DRAFT for a while to solve an issue with commits.
acid-ant force-pushed bugfix/680-export-metrics from fcd328c84c to 526ecd1432 2023-10-08 09:33:09 +00:00 Compare
acid-ant changed title from WIP: metrics: Export log and morph with script to metrics: Export log and morph with script 2023-10-08 09:33:55 +00:00
acid-ant requested review from dstepanov-yadro 2023-10-08 09:33:57 +00:00
acid-ant requested review from fyrchik 2023-10-08 09:34:06 +00:00
dstepanov-yadro approved these changes 2023-10-09 10:18:55 +00:00
fyrchik commented 2023-10-09 19:07:17 +00:00
Owner
Copy link

Please, rebase

Please, rebase
acid-ant force-pushed bugfix/680-export-metrics from 526ecd1432 to 7dba5bc442 2023-10-10 06:15:50 +00:00 Compare
acid-ant commented 2023-10-10 08:59:09 +00:00
Author
Member
Copy link

Please, rebase

Updated.

> Please, rebase Updated.
acid-ant commented 2023-10-10 09:05:45 +00:00
Author
Member
Copy link

Also, is everything OK with gRPC metrics? They must've missed last time I checked (duration of the request etc.)

Will be fixed in scope of this TrueCloudLab/frostfs-observability#4

> Also, is everything OK with gRPC metrics? They must've missed last time I checked (duration of the request etc.) Will be fixed in scope of this https://git.frostfs.info/TrueCloudLab/frostfs-observability/issues/4
acid-ant force-pushed bugfix/680-export-metrics from 7dba5bc442 to 3a997d1207 2023-10-11 14:08:13 +00:00 Compare
acid-ant added 1 commit 2023-10-12 06:37:47 +00:00
[#735] policer: Register metrics
Some checks failed
DCO action / DCO (pull_request) Successful in 3m40s
Details
Vulncheck / Vulncheck (pull_request) Successful in 3m22s
Details
Build / Build Components (1.21) (pull_request) Successful in 4m15s
Details
Tests and linters / Staticcheck (pull_request) Successful in 5m13s
Details
Tests and linters / Lint (pull_request) Successful in 5m52s
Details
Tests and linters / Tests with -race (pull_request) Failing after 6m3s
Details
Tests and linters / Tests (1.20) (pull_request) Successful in 7m28s
Details
Tests and linters / Tests (1.21) (pull_request) Successful in 7m28s
Details
Build / Build Components (1.20) (pull_request) Successful in 8m10s
Details
d5c10612f4 
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
acid-ant commented 2023-10-12 06:41:35 +00:00
Author
Member
Copy link

@fyrchik, added commit with metrics registration for policer.

@fyrchik, added commit with metrics registration for policer.
acid-ant requested review from storage-core-committers 2023-10-12 06:42:48 +00:00
fyrchik approved these changes 2023-10-12 06:50:18 +00:00
fyrchik approved these changes 2023-10-16 13:09:58 +00:00
fyrchik merged commit d5c10612f4 into master 2023-10-16 13:10:32 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
dstepanov-yadro
fyrchik
TrueCloudLab/storage-core-developers
Labels
Clear labels   
P0

Highest

  
P1

High

  
P2

Medium

  
P3

Low

  
badger

Badger DB related issues

  
frostfs-adm

   
frostfs-cli

   
frostfs-ir

   
frostfs-lens

   
frostfs-node

   
good first issue

Good for newcomers

  
triage
  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff

  
blocked

The issue or PR is blocked by something

  
bug

Something is not working

  
config

Configuration format update or breaking change

  
discussion

Talking about something in order to reach a decision or to exchange ideas

  
documentation

Documentation related

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
go

Language features adoption

  
help wanted

Extra attention is needed

  
internal

   
invalid

Something is wrong

  
kludge

This is a kludge and we know it

  
observability

Related to metrics, tracing and logs

  
perfomance

Perfomance related

  
question

More information is needed

  
refactoring

Refactoring

  
wontfix

This won't be fixed

No labels
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#718
No description provided.
Delete branch "acid-ant/frostfs-node:bugfix/680-export-metrics"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?