TrueCloudLab/frostfs-http-gw
18
1
Fork 18
Code Issues 11 Pull requests 7 Projects Releases 10 Activity Actions

[#150] Add dropped logs metric #169

Merged
alexvanin merged 1 commit from pogpp/frostfs-http-gw:feature/150_dropped_logs into master 2024-12-04 12:59:36 +00:00
Conversation 1 Commits 1 Files changed 6 +147 -91
pogpp commented 2024-11-24 10:48:09 +00:00
Member
Copy link

close #150
Signed-off-by: Pavel Pogodaev p.pogodaev@yadro.com

close #150 Signed-off-by: Pavel Pogodaev <p.pogodaev@yadro.com>
pogpp added 1 commit 2024-11-24 10:48:09 +00:00
[#150] Add dropped logs metric
All checks were successful
/ DCO (pull_request) Successful in 1m11s
Details
/ Vulncheck (pull_request) Successful in 1m29s
Details
/ Builds (pull_request) Successful in 1m54s
Details
/ Lint (pull_request) Successful in 2m51s
Details
/ Tests (pull_request) Successful in 1m55s
Details
1858668e53 
Signed-off-by: Pavel Pogodaev <p.pogodaev@yadro.com>
pogpp requested review from alexvanin 2024-11-24 10:48:10 +00:00
pogpp requested review from dkirillov 2024-11-24 10:48:10 +00:00
pogpp was assigned by dkirillov 2024-11-26 12:44:06 +00:00
dkirillov commented 2024-11-26 12:44:49 +00:00
Member
Copy link

See comments from TrueCloudLab/frostfs-s3-gw#557

See comments from https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/pulls/557
👍 1
pogpp force-pushed feature/150_dropped_logs from 1858668e53 to e9692320d6 2024-11-27 10:58:33 +00:00 Compare
pogpp force-pushed feature/150_dropped_logs from e9692320d6 to 329cff1bb1 2024-11-29 14:55:31 +00:00 Compare
dkirillov requested changes 2024-12-02 14:04:54 +00:00
Dismissed
dkirillov left a comment
Member
Copy link

Please fix integration tests (cmd/http-gw/integration_test.go)

$ make integration-test

# git.frostfs.info/TrueCloudLab/frostfs-http-gw/cmd/http-gw [git.frostfs.info/TrueCloudLab/frostfs-http-gw/cmd/http-gw.test]
cmd/http-gw/integration_test.go:105:12: assignment mismatch: 2 variables but newStdoutLogger returns 1 value
cmd/http-gw/integration_test.go:105:31: not enough arguments in call to newStdoutLogger
        have (*viper.Viper, zapcore.Level)
        want (*viper.Viper, zapcore.Level, LoggerAppSettings)
cmd/http-gw/integration_test.go:106:35: undefined: WithConfig
cmd/http-gw/integration_test.go:106:50: undefined: WithLogger
cmd/http-gw/integration_test.go:106:50: too many arguments in call to newApp
        have (context.Context, unknown type, unknown type)
        want (context.Context, *viper.Viper)
FAIL    git.frostfs.info/TrueCloudLab/frostfs-http-gw/cmd/http-gw [build failed]
?       git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/api      [no test files]
?       git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/api/layer        [no test files]
?       git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/data     [no test files]
        git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/cache            coverage: 0.0% of statements
?       git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/logs     [no test files]
        git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/handler/middleware               coverage: 0.0% of statements
        git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/net              coverage: 0.0% of statements
        git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/handler/multipart                coverage: 0.0% of statements
ok      git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/handler  0.010s  coverage: 42.0% of statements
?       git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/templates        [no test files]
        git.frostfs.info/TrueCloudLab/frostfs-http-gw/metrics           coverage: 0.0% of statements
        git.frostfs.info/TrueCloudLab/frostfs-http-gw/resolver          coverage: 0.0% of statements
        git.frostfs.info/TrueCloudLab/frostfs-http-gw/utils             coverage: 0.0% of statements
        git.frostfs.info/TrueCloudLab/frostfs-http-gw/tokens            coverage: 0.0% of statements
        git.frostfs.info/TrueCloudLab/frostfs-http-gw/response          coverage: 0.0% of statements
ok      git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/service/frostfs  0.006s  coverage: 8.5% of statements
ok      git.frostfs.info/TrueCloudLab/frostfs-http-gw/tree      0.005s  coverage: 12.9% of statements
FAIL
make: *** [Makefile:79: integration-test] Error 1
Please fix integration tests (`cmd/http-gw/integration_test.go`) ``` $ make integration-test # git.frostfs.info/TrueCloudLab/frostfs-http-gw/cmd/http-gw [git.frostfs.info/TrueCloudLab/frostfs-http-gw/cmd/http-gw.test] cmd/http-gw/integration_test.go:105:12: assignment mismatch: 2 variables but newStdoutLogger returns 1 value cmd/http-gw/integration_test.go:105:31: not enough arguments in call to newStdoutLogger have (*viper.Viper, zapcore.Level) want (*viper.Viper, zapcore.Level, LoggerAppSettings) cmd/http-gw/integration_test.go:106:35: undefined: WithConfig cmd/http-gw/integration_test.go:106:50: undefined: WithLogger cmd/http-gw/integration_test.go:106:50: too many arguments in call to newApp have (context.Context, unknown type, unknown type) want (context.Context, *viper.Viper) FAIL git.frostfs.info/TrueCloudLab/frostfs-http-gw/cmd/http-gw [build failed] ? git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/api [no test files] ? git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/api/layer [no test files] ? git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/data [no test files] git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/cache coverage: 0.0% of statements ? git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/logs [no test files] git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/handler/middleware coverage: 0.0% of statements git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/net coverage: 0.0% of statements git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/handler/multipart coverage: 0.0% of statements ok git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/handler 0.010s coverage: 42.0% of statements ? git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/templates [no test files] git.frostfs.info/TrueCloudLab/frostfs-http-gw/metrics coverage: 0.0% of statements git.frostfs.info/TrueCloudLab/frostfs-http-gw/resolver coverage: 0.0% of statements git.frostfs.info/TrueCloudLab/frostfs-http-gw/utils coverage: 0.0% of statements git.frostfs.info/TrueCloudLab/frostfs-http-gw/tokens coverage: 0.0% of statements git.frostfs.info/TrueCloudLab/frostfs-http-gw/response coverage: 0.0% of statements ok git.frostfs.info/TrueCloudLab/frostfs-http-gw/internal/service/frostfs 0.006s coverage: 8.5% of statements ok git.frostfs.info/TrueCloudLab/frostfs-http-gw/tree 0.005s coverage: 12.9% of statements FAIL make: *** [Makefile:79: integration-test] Error 1 ````
👍 1
cmd/http-gw/app.go Outdated
@ -72,1 +73,4 @@
loggerSettings struct {
mu sync.RWMutex
appMetrics *AppMetrics
dkirillov commented 2024-12-02 14:03:41 +00:00
Member
Copy link

I would use here *metrics.GateMetrics instead of *AppMetrics or access AppMetrics.provider (in loggerSettings.DroppedLogsInc respecting AppMetrics.enabled like in AppMetrics.MarkHealthy)

I would use here `*metrics.GateMetrics` instead of `*AppMetrics` or access `AppMetrics.provider` (in `loggerSettings.DroppedLogsInc` respecting `AppMetrics.enabled` like in `AppMetrics.MarkHealthy`)
👍 1
dkirillov marked this conversation as resolved
cmd/http-gw/app.go Outdated
@ -80,3 +85,1 @@
Option func(a *app)
gateMetrics struct {
AppMetrics struct {
dkirillov commented 2024-12-02 13:53:12 +00:00
Member
Copy link

Do we really need change gateMetrics to AppMetrics?

Do we really need change `gateMetrics` to `AppMetrics`?
👍 1
dkirillov marked this conversation as resolved
metrics/metrics.go Outdated
@ -76,6 +77,10 @@ type stateMetrics struct {
versionInfo *prometheus.GaugeVec
}
type StatisticMetrics struct {
dkirillov commented 2024-12-02 13:56:30 +00:00
Member
Copy link

Do we need this type be exported?

Do we need this type be exported?
👍 1
dkirillov marked this conversation as resolved
pogpp force-pushed feature/150_dropped_logs from 329cff1bb1 to 0631d7bbc1 2024-12-03 14:29:06 +00:00 Compare
alexvanin approved these changes 2024-12-03 14:55:29 +00:00
Dismissed
dkirillov reviewed 2024-12-04 12:03:14 +00:00
cmd/http-gw/server.go Outdated
@ -42,2 +44,4 @@
cert *tls.Certificate
}
Logger struct {
dkirillov commented 2024-12-04 12:03:14 +00:00
Member
Copy link

Why do we add this struct to this file?

Why do we add this struct to this file?
dkirillov approved these changes 2024-12-04 12:05:00 +00:00
Dismissed
dkirillov left a comment
Member
Copy link

LGTM, but please move Logger struct to app.go or settings.go

LGTM, but please move `Logger` struct to app.go or settings.go
pogpp force-pushed feature/150_dropped_logs from 0631d7bbc1 to e81f01c2ab 2024-12-04 12:49:36 +00:00 Compare
pogpp dismissed alexvanin's review 2024-12-04 12:49:36 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

pogpp dismissed dkirillov's review 2024-12-04 12:49:37 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

alexvanin approved these changes 2024-12-04 12:51:43 +00:00
alexvanin merged commit e81f01c2ab into master 2024-12-04 12:59:36 +00:00
alexvanin deleted branch feature/150_dropped_logs 2024-12-04 12:59:39 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
alexvanin
Labels
Clear labels   
P0

Highest

  
P1

High

  
P2

Medium

  
P3

Low

  
good first issue

Good for newcomers

  
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
good first issue
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
pogpp
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-http-gw#169
No description provided.
Delete branch "pogpp/frostfs-http-gw:feature/150_dropped_logs"

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?