Skip to content

Comments

feat: initialize prometheus and grafana for metrics#173

Open
rajpandya737 wants to merge 5 commits intomainfrom
feat/monitoring-services
Open

feat: initialize prometheus and grafana for metrics#173
rajpandya737 wants to merge 5 commits intomainfrom
feat/monitoring-services

Conversation

@rajpandya737
Copy link
Member

@rajpandya737 rajpandya737 commented Oct 29, 2025

What is this issue for and how does it solve it

Adds Prometheus and Grafana for metrics and dashboard for our application
You can view these metrics by visiting localhost:3000 and login with admin credentials

Link to the Github Issue

#167

@andrewwu13 andrewwu13 marked this pull request as ready for review November 17, 2025 20:58
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Nov 17, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🔴
Default weak credentials

Description: Grafana admin credentials are hardcoded to weak defaults ('GF_SECURITY_ADMIN_USER=admin'
and 'GF_SECURITY_ADMIN_PASSWORD=admin'), enabling trivial takeover of the monitoring
dashboard if the stack is exposed.
docker-compose.yml [61-63]

Referred Code
- GF_SECURITY_ADMIN_USER=admin
- GF_SECURITY_ADMIN_PASSWORD=admin
- GF_USERS_DEFAULT_THEME=light
Unauthenticated metrics endpoint

Description: Prometheus metrics are exposed without authentication or IP restrictions, which can leak
sensitive operational data (e.g., paths, status codes, latencies) and be abused for
reconnaissance if the endpoint is reachable externally.
main.py [39-41]

Referred Code
# Prometheus metrics
instrumentator = Instrumentator().instrument(app)
instrumentator.expose(app, include_in_schema=False, should_gzip=True)
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Plaintext secrets: Grafana admin credentials are hardcoded in environment variables within compose, risking
exposure in logs and configuration leaks.

Referred Code
- GF_SECURITY_ADMIN_USER=admin
- GF_SECURITY_ADMIN_PASSWORD=admin
- GF_USERS_DEFAULT_THEME=light

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing context: The new code does not add or modify audit logging for critical actions, and it’s unclear
whether uploads or permission changes are being logged with user and outcome context
elsewhere.

Referred Code
files_to_upload = [
    file_path
    for file_path in session_path.iterdir()
    if file_path.is_file() and file_path.name != "signature.png"
]

if not files_to_upload:
    logger.warning(

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing guards: Metrics initialization and exposure are added without visible error handling for
instrumentation failures or missing dependencies, which could impact app startup.

Referred Code
# Prometheus metrics
instrumentator = Instrumentator().instrument(app)
instrumentator.expose(app, include_in_schema=False, should_gzip=True)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Open endpoints: Prometheus and Grafana services are exposed on default ports without visible
authentication or network restrictions, which may require validation of deployment
security posture.

Referred Code
  ports:
    - "9090:9090"
  volumes:
    - ./monitoring/prometheus:/etc/prometheus
    - prometheus-data:/prometheus
  command:
    - "--config.file=/etc/prometheus/prometheus.yml"
  networks:
    - monitoring
grafana:
  image: grafana/grafana:latest
  container_name: grafana
  profiles: ["monitoring"]
  depends_on:
    - prometheus
  ports:
    - "3000:3000"
  environment:

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@andrewwu13 andrewwu13 changed the title feat: initialize LGTM stack - not working feat: initialize prometheus and grafana for metrics Nov 17, 2025
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Nov 17, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Avoid hardcoding Grafana admin credentials
Suggestion Impact:The commit replaced hardcoded GF_SECURITY_ADMIN_USER and GF_SECURITY_ADMIN_PASSWORD values with environment variables, aligning with the suggestion (though without default fallbacks). It also changed the default theme to dark.

code diff:

     environment:
-      - GF_SECURITY_ADMIN_USER=admin
-      - GF_SECURITY_ADMIN_PASSWORD=admin
-      - GF_USERS_DEFAULT_THEME=light
+      - GF_SECURITY_ADMIN_USER=${GRAFANA_ADMIN_USER}
+      - GF_SECURITY_ADMIN_PASSWORD=${GRAFANA_ADMIN_PASSWORD}
+      - GF_USERS_DEFAULT_THEME=dark

Replace hardcoded Grafana admin credentials in docker-compose.yml with
environment variables to enhance security.

docker-compose.yml [60-63]

 environment:
-  - GF_SECURITY_ADMIN_USER=admin
-  - GF_SECURITY_ADMIN_PASSWORD=admin
+  - GF_SECURITY_ADMIN_USER=${GRAFANA_ADMIN_USER:-admin}
+  - GF_SECURITY_ADMIN_PASSWORD=${GRAFANA_ADMIN_PASSWORD:-admin}
   - GF_USERS_DEFAULT_THEME=light

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a significant security vulnerability by pointing out hardcoded Grafana admin credentials and proposes a best-practice solution using environment variables.

High
High-level
Complete or remove partial OpenTelemetry implementation

The PR adds OpenTelemetry dependencies without implementing the feature. It is
suggested to either complete the implementation or remove the unused
dependencies to avoid unnecessary project bloat.

Examples:

pyproject.toml [19-21]
    "opentelemetry-sdk>=1.38.0",
    "opentelemetry-exporter-otlp>=1.38.0",
    "opentelemetry-instrumentation-fastapi>=0.59b0",

Solution Walkthrough:

Before:

# pyproject.toml
dependencies = [
    ...,
    "prometheus-fastapi-instrumentator>=7.1.0",
    "opentelemetry-sdk>=1.38.0",
    "opentelemetry-exporter-otlp>=1.38.0",
    "opentelemetry-instrumentation-fastapi>=0.59b0",
]

# src/main.py
...
app = FastAPI(...)
# Prometheus metrics are configured
instrumentator = Instrumentator().instrument(app)
instrumentator.expose(app, ...)

# No OpenTelemetry implementation exists in the codebase.

After:

# pyproject.toml
dependencies = [
    ...,
    "prometheus-fastapi-instrumentator>=7.1.0",
    # "opentelemetry-sdk>=1.38.0", (removed)
    # "opentelemetry-exporter-otlp>=1.38.0", (removed)
    # "opentelemetry-instrumentation-fastapi>=0.59b0", (removed)
]

# src/main.py
...
app = FastAPI(...)
# Prometheus metrics are configured
instrumentator = Instrumentator().instrument(app)
instrumentator.expose(app, ...)

# No OpenTelemetry implementation exists in the codebase.
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that OpenTelemetry dependencies were added without any implementation, which is a significant code quality issue that increases the project's dependency footprint for no benefit.

Medium
  • Update

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.

2 participants