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 a metric for tracking the number of pushes #953

Merged
merged 2 commits into from
May 21, 2024
Merged

Conversation

ikhoon
Copy link
Contributor

@ikhoon ikhoon commented May 2, 2024

Motivation:

It isn't easy to check how many commits have been pushed to a specific repository without looking at log files.

This PR is the orignal commit of #943 that tracks the number of pushes instead of adding project and repo tags to all api metrics. #943 has been reverted because it increased in CPU and memory usage.

Modifications:

  • Add a Counter that is increased when a commit is pushed to a repository.
    • project and repository are added as tags.

Result:

You can monitor the number of pushes to a repository using the commits.push metric.

Motivation:

It is difficult to check how many commits have been pushed to a specific
repository without looking at log files.

This PR is the orignal commit of line#943 that tracks the number of pushes
instead of adding `project` and `repo` tags to all `api` metrics.
 line#943 has been reverted because it increased in CPU and memory usage.

Modifications:

- Add a `Counter` that is increased when a commit is pushed to a
  repository.
  - `project` and `repository` are added as tags.

Result:

You can monitor the number of pushes to a repository
using the `commits.push` metric.
Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

#943 has been reverted because it increased in CPU and memory usage.

This is unfortunate as I was actually really looking forward to the metrics in that PR 😅 Maybe next time.

@ikhoon
Copy link
Contributor Author

ikhoon commented May 2, 2024

#943 has been reverted because it increased in CPU and memory usage.

This is unfortunate as I was actually really looking forward to the metrics in that PR 😅 Maybe next time.

Otherwise, we may add a filter to tag repositories selectively. The filter could be used to exclude less interesting repositories generated mechanically by a plugin or bot.

interface RepositoryMetricFilter {
   boolean shouldTagRepository(String projectName, String repoName);
}

new CentralDogmaBuilder()
   .repositoryMetricFilter((project, repo) -> {
      if (project.equals(registryProject) {
         return false;
      } else {
         return true;
      }
   });

Copy link
Member

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -190,6 +194,10 @@ public CompletableFuture<PushResultDto> push(
CommitMessageDto commitMessage,
@RequestConverter(ChangesRequestConverter.class) Iterable<Change<?>> changes) {
checkPush(repository.name(), changes);
meterRegistry.counter("commits.push",
Copy link
Member

Choose a reason for hiding this comment

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

Note: Didn't add this counter to ThriftService because it's going to be deprecated.

@minwoox minwoox merged commit 7193f13 into line:main May 21, 2024
8 of 9 checks passed
@minwoox
Copy link
Member

minwoox commented May 21, 2024

@ikhoon 👍 👍 👍

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.

None yet

3 participants