-
Notifications
You must be signed in to change notification settings - Fork 32
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
[93] fix: Connected Grafana Api to the response-time-interceptor #150
base: dev
Are you sure you want to change the base?
[93] fix: Connected Grafana Api to the response-time-interceptor #150
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis release introduces important updates across multiple files, focusing on enhancing functionality and code maintainability. Key changes include improvements in Prometheus metrics handling, updates to the response-time tracking interceptor for better Grafana integration, and addition of new utility functions for generating Grafana dashboard panels. Minor formatting updates and new unit tests for file upload interceptors are also included, along with updated configuration management using NestJS ConfigModule and new dependencies in Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AppController
participant ResponseTimeInterceptor
participant GrafanaAPI
participant PrometheusController
Client->>AppController: Send request
AppController->>ResponseTimeInterceptor: Intercept request
ResponseTimeInterceptor->>GrafanaAPI: Fetch or update dashboard
GrafanaAPI-->>ResponseTimeInterceptor: Return dashboard data
ResponseTimeInterceptor->>AppController: Proceed with request
AppController->>PrometheusController: Fetch metrics
PrometheusController->>Client: Return metrics with content-type header
Poem
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
---|---|---|---|---|---|
- | - | Grafana Service Account Token | 3b0e08e | sample/02-monitoring/src/response.interceptor.ts | View secret |
- | - | Grafana Service Account Token | 3b0e08e | sample/02-monitoring/src/response.interceptor.ts | View secret |
- | - | Grafana Service Account Token | f4ec971 | sample/02-monitoring/src/response.interceptor.ts | View secret |
- | - | Grafana Service Account Token | f4ec971 | sample/02-monitoring/src/response.interceptor.ts | View secret |
- | - | Grafana Service Account Token | 125e67d | sample/02-monitoring/src/response.interceptor.ts | View secret |
- | - | Grafana Token | 9154a6d | sample/02-monitoring/README.md | View secret |
- | - | Grafana Token | 3fa0a9c | sample/02-monitoring/README.md | View secret |
- | - | Grafana Token | 9154a6d | sample/02-monitoring/README.md | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
sample/02-monitoring/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
Files selected for processing (9)
- packages/common/src/controllers/prometheus.controller.ts (1 hunks)
- packages/common/src/interceptors/response-time.interceptor.ts (3 hunks)
- packages/common/src/interceptors/utils.ts (2 hunks)
- sample/02-monitoring/env-example (1 hunks)
- sample/02-monitoring/nest-cli.json (1 hunks)
- sample/02-monitoring/package.json (2 hunks)
- sample/02-monitoring/src/app.controller.ts (1 hunks)
- sample/02-monitoring/src/app.module.ts (1 hunks)
- sample/02-monitoring/src/main.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- sample/02-monitoring/env-example
- sample/02-monitoring/nest-cli.json
Additional context used
Biome
packages/common/src/controllers/prometheus.controller.ts
[error] 8-8: Decorators are not valid here.
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabled
option totrue
in your configuration file.(parse)
Additional comments not posted (16)
packages/common/src/controllers/prometheus.controller.ts (1)
9-9
: LGTM!The change from
response.headers
toresponse.header
is correct for setting a single header in Fastify.sample/02-monitoring/src/app.module.ts (1)
8-8
: LGTM!The addition of
ConfigModule.forRoot()
is correct for loading environment variables.sample/02-monitoring/src/main.ts (1)
8-16
: LGTM!The changes correctly fetch configuration values and use them to set up the
ResponseTimeInterceptor
.sample/02-monitoring/package.json (4)
27-27
: Approved: Addition of axios dependency.The addition of
axios
is appropriate for making HTTP requests.
30-30
: Approved: Addition of uuid dependency.The addition of
uuid
is appropriate for generating unique identifiers.
34-35
: Approved: Addition of @samagra-x/schematics and @samagra-x/stencil-cli devDependencies.The addition of
@samagra-x/schematics
and@samagra-x/stencil-cli
is appropriate for managing project schematics and CLI tools.
45-45
: Approved: Addition of husky devDependency.The addition of
husky
is appropriate for managing Git hooks.packages/common/src/interceptors/response-time.interceptor.ts (5)
5-6
: Approved: Addition of axios import.The addition of
axios
is necessary for making HTTP requests to the Grafana API.
7-12
: Approved: Addition of utility function imports.The addition of
generateBaseJSON
,generateRow
,getDashboardByUID
, andgetDashboardJSON
imports is necessary for the updated functionality of theResponseTimeInterceptor
class.
22-33
: Approved: Updates to the constructor.The constructor updates are necessary for initializing the
ResponseTimeInterceptor
with the required parameters for interacting with the Grafana API.
18-20
: Approved: Addition of private members.The addition of
dashboardUid
,grafanaBaseURL
, andapiToken
as private members is necessary for storing the Grafana API configuration and dashboard UID.
Line range hint
35-111
:
Approved: Updates to the init method.The updates to the
init
method are necessary for setting up the Grafana dashboard and adding the histogram panel.packages/common/src/interceptors/utils.ts (4)
1-2
: Approved: Addition of axios import.The addition of
axios
is necessary for making HTTP requests to the Grafana API.
3-82
: Approved: Updates to generateBaseJSON function.The updates to the
generateBaseJSON
function are necessary for generating the base JSON structure for the Grafana dashboard with the given histogram title.
Line range hint
86-113
:
Approved: Updates to generateRow function.The updates to the
generateRow
function are necessary for generating a row with multiple panels for the Grafana dashboard.
609-650
: Approved: Addition of new utility functions.The addition of
generateAverageResponseTimePanel
,getDashboardByUID
, andgetDashboardJSON
utility functions is necessary for generating specific panels and retrieving dashboard JSON from the Grafana API.
@UseInterceptors( | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty @UseInterceptors
decorator.
The @UseInterceptors
decorator is empty, which might be an oversight. Ensure that the intended interceptor is specified.
-@UseInterceptors(
-)
+@UseInterceptors(ResponseTimeInterceptor)
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
@UseInterceptors( | |
) | |
@UseInterceptors(ResponseTimeInterceptor) |
@@ -1,11 +1,12 @@ | |||
import { Controller, Get, Res } from '@nestjs/common'; | |||
import { FastifyReply } from 'fastify'; | |||
import { register } from 'prom-client'; | |||
|
|||
@Controller() | |||
export class PrometheusController { | |||
@Get('metrics') | |||
async metrics(@Res() response: FastifyReply) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Static Analysis: Decorators are not valid here.
Decorators are only valid on class declarations, class expressions, and class methods. You can enable parameter decorators by setting the unsafeParameterDecoratorsEnabled
option to true
in your configuration file.
- async metrics(@Res() response: FastifyReply) {
+ async metrics(response: FastifyReply) {
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
async metrics(@Res() response: FastifyReply) { | |
async metrics(response: FastifyReply) { |
Tools
Biome
[error] 8-8: Decorators are not valid here.
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabled
option totrue
in your configuration file.(parse)
main.ts.-.new.-.Visual.Studio.Code.2024-07-04.23-50-45.mp4Dashboards.-.Grafana.-.Google.Chrome.2024-07-04.23-51-20.mp4 |
1ce4457
to
3b0e08e
Compare
|
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
packages/user-service/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
sample/02-monitoring/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
Files selected for processing (11)
- packages/common/src/controllers/prometheus.controller.ts (1 hunks)
- packages/common/src/interceptors/file-upload.interceptor.ts (1 hunks)
- packages/common/src/interceptors/response-time.interceptor.ts (3 hunks)
- packages/common/src/interceptors/utils.ts (2 hunks)
- packages/common/test/file-upload.interceptor.spec.ts (1 hunks)
- sample/02-monitoring/env-example (1 hunks)
- sample/02-monitoring/nest-cli.json (1 hunks)
- sample/02-monitoring/package.json (2 hunks)
- sample/02-monitoring/src/app.controller.ts (1 hunks)
- sample/02-monitoring/src/app.module.ts (1 hunks)
- sample/02-monitoring/src/main.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- packages/common/src/interceptors/file-upload.interceptor.ts
- sample/02-monitoring/nest-cli.json
Files skipped from review as they are similar to previous changes (6)
- packages/common/src/interceptors/response-time.interceptor.ts
- packages/common/src/interceptors/utils.ts
- sample/02-monitoring/env-example
- sample/02-monitoring/package.json
- sample/02-monitoring/src/app.controller.ts
- sample/02-monitoring/src/main.ts
Additional comments not posted (11)
packages/common/src/controllers/prometheus.controller.ts (2)
9-9
: Change approved.The change from
response.headers
toresponse.header
is correct and aligns with the FastifyReply API.
8-8
: Static Analysis: Decorators are not valid here.Decorators are only valid on class declarations, class expressions, and class methods. You can enable parameter decorators by setting the
unsafeParameterDecoratorsEnabled
option totrue
in your configuration file.- async metrics(@Res() response: FastifyReply) { + async metrics(response: FastifyReply) {sample/02-monitoring/src/app.module.ts (2)
5-5
: Change approved.The import statement for
ConfigModule
is correct and necessary.
8-8
: Change approved.The inclusion of
ConfigModule.forRoot()
in theimports
array follows best practices for configuration management in NestJS.packages/common/test/file-upload.interceptor.spec.ts (7)
1-3
: Change approved.The import statements are correct and necessary for the tests.
5-11
: Change approved.The describe block and setup for
FastifyFileInterceptor
are correct and follow best practices.
13-15
: Change approved.The test case correctly checks if the interceptor is defined.
17-26
: Change approved.The test case correctly checks if the interceptor handles file upload.
30-44
: Change approved.The test case correctly checks if the interceptor handles errors.
47-59
: Change approved.The test case correctly checks if the interceptor handles cases where the file is not present or null.
63-73
: Change approved.The helper functions
createMockContext
andcreateMockNextHandler
are correct and follow best practices.
Parent issue: #93 |
@Savio629 CI is failing on this. |
|
test / test-sample (01-monitoring) (pull_request) is failing because package is not updated of @samagra-x/stencil with this pr changes GitGuardian Security Checks is failing as key was push in this pr
Yes |
Referene Issue : #76
Dashboards.-.Grafana.-.Google.Chrome.2024-07-04.12-07-19.mp4
Summary by CodeRabbit
New Features
axios
anduuid
packages as dependencies.ConfigModule
for environment configuration management.Improvements
metrics
endpoint.Bug Fixes
nest-cli.json
.Tests
FastifyFileInterceptor
focusing on file uploads and error scenarios.Documentation