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

Fix deploying latency monitor #2

Merged
merged 18 commits into from
Sep 13, 2024
Merged

Conversation

ausias-armesto
Copy link
Contributor

@ausias-armesto ausias-armesto commented Sep 9, 2024

  • Adds push gateway url

Summary by CodeRabbit

  • New Features

    • Enhanced application observability by integrating with a Prometheus push gateway for latency monitoring.
    • Updated configuration settings to include new environment variables for improved metrics collection, including mandatory checks for critical variables.
    • Introduced a .prettierignore file to exclude sensitive files from automated formatting.
    • Added support for dynamic labeling of metrics based on environment variables.
    • Automated the merging of pull requests and Docker image builds through new GitHub Actions workflows.
    • Implemented a manual release process with version bumping and notifications.
  • Improvements

    • Streamlined workflow configuration for the build process, including clearer steps for repository checkout, Node.js setup, and Google Cloud Platform integration.
    • Improved metrics collection process for better performance analysis and debugging.
    • Excluded Visual Studio Code settings from version control to maintain project cleanliness.

Copy link

coderabbitai bot commented Sep 9, 2024

Walkthrough

Walkthrough

The pull request introduces updates to the .env.sample, .github/workflows/build.yaml, .github/workflows/merge.yaml, .github/workflows/release.yaml, .prettierignore, .gitignore, and src/index.ts files. The .env.sample file modifies the UHTTP_LM_PUSH_GATEWAY variable to include a complete URL for a Prometheus push gateway and adds several new environment variables for enhanced metrics collection. The workflow configuration in .github/workflows/build.yaml is updated with new steps for repository checkout, Node.js setup, and Google Cloud Platform (GCP) configuration. A new workflow for merging pull requests is added in .github/workflows/merge.yaml, and another for release automation in .github/workflows/release.yaml. A new .prettierignore file is added to exclude specific JSON files from Prettier formatting, and the .gitignore file is updated to ignore the Visual Studio Code workspace settings directory. Additionally, the src/index.ts file enhances the Settings type and refactors the metrics collection logic.

Changes

File Change Summary
.env.sample Updated UHTTP_LM_PUSH_GATEWAY to a complete URL and added new environment variables for metrics.
.github/workflows/build.yaml Added steps for checking out the repository, setting up Node.js, and configuring GCP.
.github/workflows/merge.yaml Introduced a new workflow for automating PR merges with steps for repository checkout, Node.js, and GCP setup.
.github/workflows/release.yaml Added a new workflow for manual release automation, including build, lint, test, and version bumping steps.
.prettierignore Introduced file to ignore gha-creds-*.json in Prettier formatting.
.gitignore Added entry to ignore .vscode/ directory.
src/index.ts Enhanced Settings type and refactored metrics collection logic to support dynamic labels.

Assessment against linked issues

Objective Addressed Explanation
Update configuration for metrics collection (undefined)
Improve CI/CD workflow for releases (undefined)
Automate PR merging process (undefined)

Possibly related issues

  • None identified.

Possibly related PRs

  • Allow continuous execution #1: The changes in the .env.sample file in both the main PR and the retrieved PR involve modifications to the UHTTP_LM_PUSH_GATEWAY variable, indicating a direct connection in terms of environment variable configuration for the latency monitoring application.

Tip

OpenAI O1 model for chat
  • We have deployed OpenAI's latest O1 model for chat.
  • OpenAI claims that this model has superior reasoning capabilities than their GPT-4o model.
  • Please share any feedback with us in the discussions post.

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1dcb2c6 and 73900b3.

Files selected for processing (1)
  • src/index.ts (4 hunks)
Additional comments not posted (11)
src/index.ts (11)

11-11: LGTM!

The addition of the pushGateway property to the Settings type is necessary to configure the Prometheus push gateway URL. This change aligns with the PR objective.


14-15: The code has been updated to use the Record syntax for the metricLabels and metrics properties as suggested in the past review comments. Therefore, the past comments are no longer applicable.


39-40: LGTM!

Throwing an error if the UHTTP_LM_PUSH_GATEWAY environment variable is missing is a good practice to ensure that the application is properly configured before starting. This change aligns with the PR objective.


42-63: LGTM!

Adding checks for the presence of critical metrics-related environment variables is a good practice to ensure that the application is properly configured before starting. This change enhances the robustness of the application by preventing runtime errors due to missing configurations.


86-99: LGTM!

The initialization of the settings object has been updated to populate the new metrics and metricLabels structures based on the environment variables. This change improves the flexibility of the metrics collection by allowing for dynamic and contextual metrics collection.


105-146: LGTM!

The initialization of the metrics object with instances of prom.Summary and prom.Counter for different metrics allows for detailed performance tracking. The use of labelNames derived from the keys of the metricLabels object ensures that the metrics are collected with the appropriate labels dynamically.


169-170: LGTM!

The refactoring of the collectMetrics and reportError functions to work with the new metrics structure allows them to observe metrics with the appropriate labels dynamically. This change aligns with the updates made to the Settings type and the initialization of the metrics object.


174-180: LGTM!

The update to the collectMetrics function to accept a metrics object of type Record<string, prom.Summary> and observe the metrics with the appropriate labels dynamically aligns with the updates made to the Settings type and the initialization of the metrics object. This change allows for more granular and context-aware metrics collection.


184-187: LGTM!

The update to the reportError function to accept an errorCounter object of type prom.Counter and increment the error counter when an error occurs aligns with the updates made to the Settings type and the initialization of the metrics object. This change allows for tracking errors with the appropriate labels dynamically.


191-195: LGTM!

The update to the pushMetrics function to use the pushGateway and metricLabels properties from the settings object aligns with the updates made to the Settings type and the initialization of the settings object. This change ensures that the metrics are pushed to the configured push gateway with the appropriate labels.


197-200: LGTM!

The update to the success and error logging statements in the pushMetrics function to include the version and push gateway URL provides more context in the logs, which can be helpful for debugging and monitoring purposes.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)
.env.sample (1)

7-7: LGTM! The change enhances observability.

The change to the UHTTP_LM_PUSH_GATEWAY variable is approved.

Pushing latency monitoring data to a Prometheus push gateway is a good practice for enhancing observability. It allows you to collect and analyze metrics from your application, which can help in identifying performance bottlenecks and optimizing the application.

A few additional suggestions:

  1. Consider setting up alerts based on the latency metrics to proactively detect and resolve performance issues.
  2. Ensure that the Prometheus server is configured to scrape data from the push gateway at an appropriate interval.
  3. Regularly monitor the disk usage of the Prometheus server to avoid running out of storage space due to the accumulation of metrics data over time.
  4. Explore visualizing the latency metrics using a tool like Grafana to gain better insights into the application's performance.
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d273083 and 14e84ed.

Files selected for processing (1)
  • .env.sample (1 hunks)

@ausias-armesto ausias-armesto changed the title Test deploying latency monitor Fix deploying latency monitor Sep 10, 2024
@ausias-armesto ausias-armesto self-assigned this Sep 10, 2024
@ausias-armesto ausias-armesto added bug Something isn't working devops labels Sep 10, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Outside diff range, codebase verification and nitpick comments (3)
.github/workflows/merge.yaml (1)

1-45: LGTM!

The workflow configuration looks good. It defines a workflow for merging pull requests, triggered when a pull request is closed and merged into the main branch. The workflow runs on a self-hosted runner, checks out the repository, sets up Node.js and GCP, and builds and pushes a Docker image.

The use of reusable actions from the hoprnet/hopr-workflows repository is a good practice for maintaining consistency across workflows.

The static analysis tool flagged the self-hosted runner label as unknown. If this is a custom label for a self-hosted runner, consider adding it to the actionlint.yaml config file to suppress the warning:

# actionlint.yaml
runner_labels:
  - self-hosted-hoprnet-small
Tools
actionlint

17-17: label "self-hosted-hoprnet-small" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "macos-11", "macos-11.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

.github/workflows/build.yaml (1)

17-30: Workflow configuration changes look good!

The changes to the workflow configuration improve the clarity and functionality by explicitly defining the environment setup steps, such as checking out the repository, setting up Node.js, and setting up GCP. This enhances the overall build process.

The static analysis tool flagged the self-hosted runner label as unknown. If this is a custom label for a self-hosted runner, consider adding it to the actionlint.yaml config file to suppress the warning:

# actionlint.yaml
runner_labels:
  - self-hosted-hoprnet-small
.github/workflows/release.yaml (1)

1-108: Release workflow configuration looks good!

The new workflow configuration defines a comprehensive process for closing a release, including building the project, running tests, creating a release, pushing a Docker image, bumping the version, creating a pull request, and notifying about the new release. The use of reusable actions from the hoprnet/hopr-workflows repository is a good practice for maintaining consistency across workflows.

The static analysis tool flagged the self-hosted runner label as unknown. If this is a custom label for a self-hosted runner, consider adding it to the actionlint.yaml config file to suppress the warning:

# actionlint.yaml
runner_labels:
  - self-hosted-hoprnet-small
Tools
actionlint

23-23: label "self-hosted-hoprnet-small" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "macos-11", "macos-11.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


60-60: shellcheck reported issue in this script: SC2086:info:2:46: Double quote to prevent globbing and word splitting

(shellcheck)


80-80: shellcheck reported issue in this script: SC2086:info:3:40: Double quote to prevent globbing and word splitting

(shellcheck)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between fbacd5a and afd4b83.

Files selected for processing (3)
  • .github/workflows/build.yaml (2 hunks)
  • .github/workflows/merge.yaml (1 hunks)
  • .github/workflows/release.yaml (1 hunks)
Additional context used
actionlint
.github/workflows/merge.yaml

17-17: label "self-hosted-hoprnet-small" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "macos-11", "macos-11.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

.github/workflows/build.yaml

14-14: label "self-hosted-hoprnet-small" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "macos-11", "macos-11.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

.github/workflows/release.yaml

23-23: label "self-hosted-hoprnet-small" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "macos-11", "macos-11.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


60-60: shellcheck reported issue in this script: SC2086:info:2:46: Double quote to prevent globbing and word splitting

(shellcheck)


80-80: shellcheck reported issue in this script: SC2086:info:3:40: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/release.yaml Show resolved Hide resolved
Copy link
Member

@esterlus esterlus left a comment

Choose a reason for hiding this comment

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

Tested it and it works great.

Unfortunately we cannot really print the payload pushed to the push gateway with a reasonable effort.

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
Copy link
Member

@esterlus esterlus left a comment

Choose a reason for hiding this comment

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

Great

@ausias-armesto ausias-armesto merged commit cd6b01c into main Sep 13, 2024
2 checks passed
@ausias-armesto ausias-armesto deleted the ausias/deploy-latency-monitor branch September 18, 2024 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working devops
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants