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

Add metrics for principal cache memory usage #42

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

fum1h1to
Copy link
Contributor

@fum1h1to fum1h1to commented Aug 14, 2024

Description

Add memoryUsage for principal cache

Type of change

  • Bug fix
  • New feature
  • Refactoring (no functional changes, no api changes)
  • Non-code changes (update documentation, pipeline, etc.)

Flags

  • Breaks backward compatibility
  • Requires a documentation update
  • Has untestable code

Related issue/PR

Delete this section if there are no issues or pull requests that relate to this pull request.

  • Fixes #issue
  • Closes #PR

Checklist

  • Followed the guidelines in the CONTRIBUTING document
  • Added prefix [skip ci]/[ci skip]/[no ci]/[skip actions]/[actions skip] in the PR title if necessary
  • Tested and linted the code
  • Commented the code
  • Made corresponding changes to the documentation
  • Passed all pipeline checking

Checklist for maintainer

  • Use Squash and merge
  • Double-confirm the merge message has prefix [skip ci]/[ci skip]/[no ci]/[skip actions]/[actions skip]
  • Delete the branch after merge

Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
@fum1h1to fum1h1to force-pushed the feature/add-principal-cache-metrics branch from 1cf5e27 to a10d2fa Compare September 10, 2024 06:39
Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
@fum1h1to fum1h1to marked this pull request as ready for review September 11, 2024 09:31
Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
Copy link
Contributor

@y-myajima y-myajima left a comment

Choose a reason for hiding this comment

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

metrics.goに追加した関数に関するテストも追加をお願いします


// Collect is implementation of prometheus.Collector.Collect
func (m *metrics) Collect(ch chan<- prometheus.Metric) {
// m.mutex.RLock()
Copy link
Contributor

Choose a reason for hiding this comment

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

ここは必要ですか?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix: d0c67da

for _, opt := range opts {
opt(m)
}

err := prometheus.Register(m.httpOriginLatency)
if err != nil {
return nil, errors.Wrap(err, "cannot register metrics")
Copy link
Contributor

Choose a reason for hiding this comment

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

なんのメトリクスの登録に失敗したかわかるようにしてください

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix: d0c67da

err := prometheus.Register(m.httpOriginLatency)
if err != nil {
return nil, errors.Wrap(err, "cannot register metrics")
}

err = prometheus.Register(m)
if err != nil {
return nil, errors.Wrap(err, "cannot register metrics")
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix: d0c67da

Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
@fum1h1to
Copy link
Contributor Author

metrics.goに追加した関数に関するテストも追加をお願いします

92aec89 で追加

Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
Signed-off-by: fum1h1to <75571344+fum1h1to@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants