Skip to content

Conversation

@mdelapenya
Copy link
Member

  • chore(grafana): use Run function
  • chore: add test for endpoints

What does this PR do?

Use the Run function in the Grafana LGTM module

Why is it important?

Migrate modules to the new API

Related issues

@mdelapenya mdelapenya requested a review from a team as a code owner October 3, 2025 15:40
@mdelapenya mdelapenya added the chore Changes that do not impact the existing functionality label Oct 3, 2025
@mdelapenya mdelapenya self-assigned this Oct 3, 2025
@mdelapenya mdelapenya added the chore Changes that do not impact the existing functionality label Oct 3, 2025
@netlify
Copy link

netlify bot commented Oct 3, 2025

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 5809c8b
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-go/deploys/68dfee76d75de20008387ab3
😎 Deploy Preview https://deploy-preview-3412--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Oct 3, 2025

Summary by CodeRabbit

  • New Features

    • None
  • Refactor

    • Streamlined container startup with unified port exposure and readiness checks using a time-bound wait, improving reliability and predictability.
    • Enhanced error messaging for clearer diagnostics when startup fails.
  • Tests

    • Added endpoint availability checks for Loki, Tempo, OTLP (HTTP/GRPC), Prometheus, and Grafana to ensure all services are reachable after startup.

Walkthrough

Refactors Grafana LGTM module to use testcontainers.Run with module options (exposed ports, wait strategy with 2-minute deadline), updates error handling, and wraps returned container. Tests expanded to validate reachability of multiple service endpoints (Loki, Tempo, OTLP HTTP/GRPC, Prometheus, Grafana) via TCP dial checks.

Changes

Cohort / File(s) Summary
Module runtime refactor
modules/grafana-lgtm/grafana.go
Replaces GenericContainer flow with testcontainers.Run using assembled moduleOpts (WithExposedPorts, WithWaitStrategyAndDeadline). Adds time import, consolidates options, updates error wrapping, and conditionally wraps returned container.
Expanded endpoint availability tests
modules/grafana-lgtm/grafana_test.go
Adds TCP reachability checks for Loki, Tempo, OTLP HTTP/GRPC, Prometheus, and Grafana endpoints. Introduces availableURL helper, updates test naming, and uses Must*Endpoint helpers to assert non-empty endpoints then dial.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant T as Test
  participant M as GrafanaLGTM Module
  participant TC as testcontainers.Run
  participant D as Docker Engine
  participant C as Container

  T->>M: Run(ctx, opts...)
  Note over M: Build moduleOpts<br/>- WithExposedPorts(...)\n- WithWaitStrategy(ports, 2m deadline)\n- Append caller opts
  M->>TC: Run(ctx, image, moduleOpts...)
  TC->>D: Create & start container
  rect rgb(240, 248, 255)
    note right of TC: Wait until ports ready or deadline
  end
  D-->>TC: Container handle or error
  alt success
    TC-->>M: C
    M-->>T: GrafanaLGTMContainer(C)
  else error
    TC-->>M: error
    M-->>T: error "run grafana lgtm: ..."
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • stevenh

Poem

I nudge the ports—tap tap, are you awake?
Two minutes tick; I sip a dew-drop shake.
Loki hums, Tempo taps a beat,
OTLP drums, Prometheus keeps it neat.
Grafana lights—hops of pure delight!
Ship it, whiskers twitching—green tonight. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely states the primary change of migrating the Grafana LGTM module to use the Run function, which directly reflects the central update in the pull request.
Description Check ✅ Passed The pull request description directly addresses the use of the Run function in the Grafana module and the addition of endpoint tests, and it explains both what was changed and why, making it clearly related to the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5941323 and 5809c8b.

📒 Files selected for processing (2)
  • modules/grafana-lgtm/grafana.go (2 hunks)
  • modules/grafana-lgtm/grafana_test.go (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
modules/grafana-lgtm/grafana_test.go (1)
modules/grafana-lgtm/grafana.go (1)
  • Run (28-62)
modules/grafana-lgtm/grafana.go (2)
options.go (3)
  • ContainerCustomizer (22-24)
  • WithExposedPorts (454-459)
  • WithWaitStrategyAndDeadline (376-382)
modules/elasticsearch/elasticsearch.go (1)
  • Run (45-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (go)
🔇 Additional comments (6)
modules/grafana-lgtm/grafana.go (2)

6-6: LGTM!

The time import is correctly added to support the 2-minute deadline in the wait strategy.


29-51: LGTM! Clean refactoring to the new API.

The refactoring follows the established pattern from other modules (e.g., Elasticsearch) and includes:

  • Comprehensive port exposure for all 6 service endpoints
  • Robust wait strategy with multiple conditions and a 2-minute deadline
  • Proper error handling with context wrapping
  • Correct option ordering (module defaults followed by caller overrides)

The implementation is correct and well-structured.

modules/grafana-lgtm/grafana_test.go (4)

7-7: LGTM!

The net import is correctly added to support TCP connection checks in the new endpoint availability tests.


27-27: LGTM!

Renaming to "right-version" improves consistency with the newly added subtests.


51-62: LGTM!

The helper function correctly:

  • Marks itself as a test helper with t.Helper()
  • Establishes a TCP connection to verify endpoint availability
  • Safely handles cleanup with a nil check in the defer block
  • Asserts connection success with require.NoError

This is an appropriate availability check for the endpoints exposed by the Grafana LGTM stack.


64-98: LGTM! Comprehensive endpoint coverage.

The new subtests provide thorough validation of all 6 service endpoints exposed by the Grafana LGTM stack:

  • Loki (3100)
  • Tempo (3200)
  • OTLP HTTP (4318)
  • OTLP gRPC (4317)
  • Prometheus (9090)
  • Grafana (3000)

Each test follows a consistent pattern: retrieve endpoint, verify non-empty, and check TCP availability. This aligns well with the wait strategy defined in grafana.go and ensures the container is fully functional before tests complete.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mdelapenya mdelapenya merged commit 924dab6 into testcontainers:main Oct 3, 2025
17 checks passed
@mdelapenya mdelapenya deleted the use-run-grafana branch October 3, 2025 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Changes that do not impact the existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant