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

implement malloc metrics #2161

Merged
merged 28 commits into from
Dec 18, 2023
Merged

implement malloc metrics #2161

merged 28 commits into from
Dec 18, 2023

Conversation

kmrmt
Copy link
Contributor

@kmrmt kmrmt commented Aug 29, 2023

Description:

  • internal/observability/metrics/mem package has implementation of metrics
  • malloc_info metrics needs CGO_ENABLED=1, so I passed the implementation this time

Related Issue:

NONE

Versions:

  • Go Version: 1.20.5
  • Docker Version: 24.0.5
  • Kubernetes Version: v1.27.4
  • NGT Version: 2.0.16

Checklist:

Special notes for your reviewer:

@vdaas-ci
Copy link
Collaborator

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

return nil
}
tests := []test{
// TODO test cases
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
internal/core/malloc/malloc_test.go:37: Line contains TODO/BUG/FIXME: "TODO test cases" (godox)

}, nil
}

func (mm *mallocMetrics) Register(m metrics.Meter) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
ST1016: methods on the same type should have the same receiver name (seen 1x "m", 1x "mm") (stylecheck)

@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (9b6217b) 29.80% compared to head (8a7545d) 30.03%.

Files Patch % Lines
internal/observability/observability.go 0.00% 1 Missing ⚠️
pkg/agent/core/ngt/usecase/agentd.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2161      +/-   ##
==========================================
+ Coverage   29.80%   30.03%   +0.23%     
==========================================
  Files         372      371       -1     
  Lines       36372    36089     -283     
==========================================
- Hits        10840    10839       -1     
+ Misses      25019    24737     -282     
  Partials      513      513              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 29, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8a7545d
Status: ✅  Deploy successful!
Preview URL: https://cb44077a.vald.pages.dev
Branch Preview URL: https://feature-metrics-malloc-info.vald.pages.dev

View logs

@kmrmt kmrmt force-pushed the feature/metrics/malloc_info branch from 7c60cb8 to f77d5e4 Compare August 29, 2023 04:13
@kmrmt kmrmt marked this pull request as ready for review August 29, 2023 10:34
@kmrmt kmrmt changed the title [WIP] implement malloc metrics implement malloc metrics Aug 29, 2023
@kmrmt kmrmt force-pushed the feature/metrics/malloc_info branch 3 times, most recently from 40bf9ac to 5984b75 Compare September 7, 2023 01:04
@kmrmt kmrmt force-pushed the feature/metrics/malloc_info branch 2 times, most recently from 4326104 to 7a751e6 Compare September 11, 2023 05:22
deepsource-autofix bot added a commit that referenced this pull request Dec 12, 2023
This commit fixes the style issues introduced in d9e1940 according to the output
from Gofumpt and Prettier.

Details: #2161
@kmrmt kmrmt force-pushed the feature/metrics/malloc_info branch from 3ad5912 to 94ff86c Compare December 12, 2023 07:05
deepsource-autofix bot added a commit that referenced this pull request Dec 12, 2023
This commit fixes the style issues introduced in 94ff86c according to the output
from Gofumpt and Prettier.

Details: #2161
@github-actions github-actions bot removed the size/XL label Dec 12, 2023
numGC.Observe(ctx, int64(mstats.NextGC))
numGC.Observe(ctx, int64(mstats.NumGC))
numForcedGC.Observe(ctx, int64(mstats.NumForcedGC))

Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
File is not gofumpt-ed (gofumpt)

numForcedGCMetricsDescription = "The number of GC cycles called by the application"

heapWillReturnMetricsName = "heap_will_return_bytes"
heapWillReturnMetricsDescription = "Bytes of returning to OS. It contains the two following parts (heapWillReturn = heapIdle - heapReleased)"
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
G101: Potential hardcoded credentials (gosec)

case strings.HasPrefix(line, "VmPeak"):
f, err := strconv.ParseInt(fields[1], 10, 64)
if err == nil {
vmpeak.Observe(ctx, f*1024)
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
mnd: Magic number: 1024, in detected (gomnd)

case strings.HasPrefix(line, "VmSize"):
f, err := strconv.ParseInt(fields[1], 10, 64)
if err == nil {
vmsize.Observe(ctx, f*1024)
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
mnd: Magic number: 1024, in detected (gomnd)

case strings.HasPrefix(line, "VmHWM"):
f, err := strconv.ParseInt(fields[1], 10, 64)
if err == nil {
vmhwm.Observe(ctx, f*1024)
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
mnd: Magic number: 1024, in detected (gomnd)

@@ -246,6 +415,12 @@
return err
}

looksup, err := m.AsyncInt64().Gauge(
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
ineffectual assignment to err (ineffassign)

}, nil
}

func (p *procStatusMetrics) Register(m metrics.Meter) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
Function name: Register, Cyclomatic Complexity: 39, Halstead Volume: 4841.03, Maintainability Index: 18 (maintidx)

case strings.HasPrefix(line, "VmPeak"):
f, err := strconv.ParseInt(fields[1], 10, 64)
if err == nil {
vmpeak.Observe(ctx, f*1024)
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
mnd: Magic number: 1024, in detected (gomnd)

case strings.HasPrefix(line, "VmSize"):
f, err := strconv.ParseInt(fields[1], 10, 64)
if err == nil {
vmsize.Observe(ctx, f*1024)
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
mnd: Magic number: 1024, in detected (gomnd)

case strings.HasPrefix(line, "VmHWM"):
f, err := strconv.ParseInt(fields[1], 10, 64)
if err == nil {
vmhwm.Observe(ctx, f*1024)
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
mnd: Magic number: 1024, in detected (gomnd)

}, nil
}

func (p *procStatusMetrics) Register(m metrics.Meter) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
Function name: Register, Cyclomatic Complexity: 39, Halstead Volume: 4841.03, Maintainability Index: 18 (maintidx)

@kmrmt kmrmt requested review from a team, kpango and ykadowak and removed request for a team December 12, 2023 08:32
kmrmt pushed a commit that referenced this pull request Dec 12, 2023
This commit fixes the style issues introduced in 94ff86c according to the output
from Gofumpt and Prettier.

Details: #2161
kmrmt and others added 16 commits December 18, 2023 12:07
Signed-off-by: Kosuke Morimoto <[email protected]>
This commit fixes the style issues introduced in 6dfa2c8 according to the output
from Gofumpt and Prettier.

Details: #2161
Signed-off-by: Kosuke Morimoto <[email protected]>
Signed-off-by: Kosuke Morimoto <[email protected]>
Signed-off-by: Kosuke Morimoto <[email protected]>
Signed-off-by: Kosuke Morimoto <[email protected]>
Signed-off-by: Kosuke Morimoto <[email protected]>
This commit fixes the style issues introduced in 8c29a17 according to the output
from Gofumpt and Prettier.

Details: #2161
Signed-off-by: Kosuke Morimoto <[email protected]>
Signed-off-by: Kosuke Morimoto <[email protected]>
Signed-off-by: Kosuke Morimoto <[email protected]>
Signed-off-by: Kosuke Morimoto <[email protected]>
Signed-off-by: Kosuke Morimoto <[email protected]>
This commit fixes the style issues introduced in c6e1311 according to the output
from Gofumpt and Prettier.

Details: #2161
Signed-off-by: Kosuke Morimoto <[email protected]>
This commit fixes the style issues introduced in c8b302e according to the output
from Gofumpt and Prettier.

Details: #2161
@kmrmt kmrmt force-pushed the feature/metrics/malloc_info branch from bf35601 to 7b2460b Compare December 18, 2023 03:07
Signed-off-by: Kosuke Morimoto <[email protected]>
@kmrmt kmrmt requested review from a team, vankichi and hlts2 and removed request for kpango, ykadowak and a team December 18, 2023 05:26
Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM!!

@hlts2 hlts2 merged commit ceb04ea into main Dec 18, 2023
97 of 99 checks passed
@hlts2 hlts2 deleted the feature/metrics/malloc_info branch December 18, 2023 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants