Skip to content

feat(custom-issuer-go): add health endpoints and OpenTelemetry metrics#91

Merged
AdriaCarrera merged 6 commits intomainfrom
feat/custom-issuer-go-health-metrics
Apr 9, 2026
Merged

feat(custom-issuer-go): add health endpoints and OpenTelemetry metrics#91
AdriaCarrera merged 6 commits intomainfrom
feat/custom-issuer-go-health-metrics

Conversation

@kpitapeersyst
Copy link
Copy Markdown
Contributor

@kpitapeersyst kpitapeersyst commented Mar 20, 2026

feat(custom-issuer-go): add health endpoints and OpenTelemetry metrics

Changes

apps/custom-issuer-go

  • Added health endpoints:
    • GET /livez - liveness probe, always returns 200 OK
    • GET /readyz - readiness probe, checks validation keys are loaded and KMS signing capability (via Ping() with 3s timeout), returns 200 OK or 503 Service Unavailable with per-check JSON detail
  • Added HasValidationKeys() method to IssuerService
  • Added Ping(ctx) method to KMSService for health-check signing probe
  • Added OpenTelemetry metrics (OTel SDK + Prometheus exporter):
    • GET /metrics endpoint serving Prometheus-formatted metrics
    • custom_issuer_go_tokens_issued_total (counter) - successfully issued tokens
    • custom_issuer_go_tokens_failed_total (counter) - failed token issuance attempts (auth/internal errors)
    • custom_issuer_go_tokens_validation_failed_total (counter) - requests rejected due to validation errors, labeled by reason (invalid_body, jwt_empty, jwt_too_long, sign_payload_empty)
    • custom_issuer_go_issue_duration_seconds (histogram) - token issuance duration with explicit buckets (10ms-1s)
  • Added otelhttp instrumentation on application routes (skips /metrics, /livez, /readyz)

infra/chart

  • Added Prometheus scrape annotations (prometheus.io/scrape, /port, /path) to the service
  • Added livenessProbe and readinessProbe to the deployment targeting /livez and /readyz

Summary by CodeRabbit

  • New Features

    • Added liveness (/livez) and readiness (/readyz) endpoints and a Prometheus /metrics endpoint.
  • Metrics

    • Began recording token issuance metrics: success/failure counters and issuance duration histogram.
    • HTTP requests instrumented for tracing, excluding metrics and health endpoints.
  • Behavior

    • Readiness now checks signing capability and validation-key availability.
    • Improved shutdown sequence to stop server then modules, ensuring all modules are stopped.
  • Chores

    • Updated dependencies to enable telemetry and metrics integration.

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
fast-auth-spa Ready Ready Preview, Comment Apr 9, 2026 11:53am

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 20, 2026

📝 Walkthrough

Walkthrough

Adds metrics and health modules, instruments HTTP handlers with OpenTelemetry/Prometheus, exposes /metrics, /livez, /readyz, adds health probe methods on issuer and KMS services, and updates Go module dependencies for Prometheus/OTel.

Changes

Cohort / File(s) Summary
Dependency Management
apps/custom-issuer-go/go.mod
Bumped Go toolchain and added Prometheus & OpenTelemetry dependencies plus related transitive updates.
Bootstrap
apps/custom-issuer-go/main.go
Registered metrics.Module{} and health.Module{} and changed shutdown flow to wait for module stop completion.
Metrics module
apps/custom-issuer-go/modules/metrics/module.go
New metrics module: creates OTEL Prometheus exporter, sets global MeterProvider, exposes GET /metrics, handles provider shutdown, and exposes Initialized() flag.
Issuer metrics
apps/custom-issuer-go/modules/issuer/metrics/metrics.go, apps/custom-issuer-go/modules/issuer/handler/issuer.go
New OpenTelemetry metrics (counters, histogram); issuance handler records duration and increments success/failure/validation-failure counters.
Health module
apps/custom-issuer-go/modules/health/module.go, .../service/service.go, .../handler/handler.go
New health module: builds HealthService from issuer/kms modules; HealthService.CheckHealth probes issuer validation keys and KMS Ping; handler exposes /livez (200) and /readyz (200/503 JSON).
Issuer module/service
apps/custom-issuer-go/modules/issuer/module.go, .../service/service.go
Exported issuer module field (Service) and added IssuerService.HasValidationKeys() used by health checks.
KMS service
apps/custom-issuer-go/modules/kms/service/service.go
Added KMSService.Ping(ctx) performing a SHA-256 digest + KMS Sign for health probing.
AppModules lifecycle
apps/custom-issuer-go/modules/common/modules/modules.go
Stop now iterates in reverse order and aggregates stop errors with errors.Join.
Server instrumentation
apps/custom-issuer-go/server.go
Wrapped HTTP handler with otelhttp.NewHandler and filtered instrumentation for /metrics, /livez, /readyz.
Kubernetes chart
infra/chart/templates/custom-issuer-go.yaml
Added Prometheus scrape annotations and container liveness/readiness probes for /livez and /readyz.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant HealthHandler
    participant HealthService
    participant IssuerService
    participant KMSService

    Client->>HealthHandler: GET /readyz
    HealthHandler->>HealthService: CheckHealth(ctx)
    HealthService->>IssuerService: HasValidationKeys()
    IssuerService-->>HealthService: bool
    HealthService->>KMSService: Ping(ctx with 3s timeout)
    KMSService->>KMSService: SHA256("healthcheck") + KMS Sign
    KMSService-->>HealthService: error?/ok
    HealthService-->>HealthHandler: ReadyzResponse{Status, Checks}
    HealthHandler-->>Client: 200 OK / 503 Service Unavailable (JSON)
Loading
sequenceDiagram
    participant Client
    participant IssuerHandler
    participant IssuerService
    participant IssuerMetrics

    Client->>IssuerHandler: POST /issue
    IssuerHandler->>IssuerHandler: record start time
    IssuerHandler->>IssuerService: Issue(...)
    alt success
        IssuerService-->>IssuerHandler: token
        IssuerHandler->>IssuerMetrics: TokensIssuedTotal.Add(1)
        IssuerHandler->>IssuerMetrics: IssueDurationSeconds.Record(duration)
        IssuerHandler-->>Client: 200 JSON token
    else validation failure
        IssuerHandler->>IssuerMetrics: TokensValidationFailedTotal.Add(1, reason)
        IssuerHandler-->>Client: 4xx response
    else service error
        IssuerService-->>IssuerHandler: error
        IssuerHandler->>IssuerMetrics: TokensFailedTotal.Add(1)
        IssuerHandler-->>Client: error response
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

feature

Suggested reviewers

  • AgustinMJ
  • gonzalomarcote
  • AdriaCarrera
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The PR description covers all major changes with clear section headings, comprehensive change details, and endpoint/metric specifications.
Title check ✅ Passed The PR title accurately summarizes the main changes: adding health endpoints (/livez, /readyz) and OpenTelemetry metrics instrumentation to the custom-issuer-go application, which are the primary objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/custom-issuer-go-health-metrics

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

@socket-security
Copy link
Copy Markdown

socket-security Bot commented Mar 20, 2026

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report

Copy link
Copy Markdown

@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.

🧹 Nitpick comments (1)
apps/custom-issuer-go/modules/issuer/metrics/metrics.go (1)

15-18: Consider custom histogram buckets for better latency granularity.

The histogram uses Prometheus default buckets (5ms to 10s), which may not provide optimal resolution for token issuance latency. If issuance typically completes in tens of milliseconds, most observations could cluster in the first bucket.

♻️ Optional: Add custom buckets tuned for expected latency
 	IssueDurationSeconds = prometheus.NewHistogram(prometheus.HistogramOpts{
 		Name: "custom_issuer_go_issue_duration_seconds",
 		Help: "Duration of token issuance in seconds",
+		Buckets: []float64{0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1.0, 2.5},
 	})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/custom-issuer-go/modules/issuer/metrics/metrics.go` around lines 15 -
18, The histogram IssueDurationSeconds currently uses Prometheus defaults;
update the prometheus.NewHistogram call (prometheus.HistogramOpts for
IssueDurationSeconds) to add a custom Buckets slice tuned to expected token
issuance latency (e.g., tighter millisecond ranges such as 1ms, 2ms, 5ms, 10ms,
20ms, 50ms, 100ms, 200ms, 500ms, 1s) so observations don’t all fall into the
first bucket—modify the HistogramOpts to include a Buckets: []float64{...}
appropriate for tens-of-milliseconds latencies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/custom-issuer-go/modules/issuer/metrics/metrics.go`:
- Around line 15-18: The histogram IssueDurationSeconds currently uses
Prometheus defaults; update the prometheus.NewHistogram call
(prometheus.HistogramOpts for IssueDurationSeconds) to add a custom Buckets
slice tuned to expected token issuance latency (e.g., tighter millisecond ranges
such as 1ms, 2ms, 5ms, 10ms, 20ms, 50ms, 100ms, 200ms, 500ms, 1s) so
observations don’t all fall into the first bucket—modify the HistogramOpts to
include a Buckets: []float64{...} appropriate for tens-of-milliseconds
latencies.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f9d4e5dc-309f-43b8-826b-444dde50c614

📥 Commits

Reviewing files that changed from the base of the PR and between 3aa3a2b and a915a8d.

⛔ Files ignored due to path filters (1)
  • apps/custom-issuer-go/go.sum is excluded by !**/*.sum
📒 Files selected for processing (11)
  • apps/custom-issuer-go/go.mod
  • apps/custom-issuer-go/main.go
  • apps/custom-issuer-go/modules/health/handler/handler.go
  • apps/custom-issuer-go/modules/health/module.go
  • apps/custom-issuer-go/modules/health/service/service.go
  • apps/custom-issuer-go/modules/issuer/handler/issuer.go
  • apps/custom-issuer-go/modules/issuer/metrics/metrics.go
  • apps/custom-issuer-go/modules/issuer/module.go
  • apps/custom-issuer-go/modules/issuer/service/service.go
  • apps/custom-issuer-go/modules/kms/service/service.go
  • apps/custom-issuer-go/modules/metrics/module.go
📜 Review details
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-03-05T14:31:30.013Z
Learnt from: kpitapeersyst
Repo: Peersyst/fast-auth PR: 78
File: apps/custom-issuer-go/go.mod:1-1
Timestamp: 2026-03-05T14:31:30.013Z
Learning: In the Peersyst/fast-auth repository, the Go module at `apps/custom-issuer-go/go.mod` intentionally declares `module github.com/peersyst/fast-auth/apps/custom-issuer` (without the `-go` suffix). The directory is named `custom-issuer-go` temporarily while the NestJS version of the app still exists; it will be renamed to `custom-issuer` once NestJS is removed. Do not flag this module path / directory name mismatch as an issue.

Applied to files:

  • apps/custom-issuer-go/main.go
  • apps/custom-issuer-go/modules/issuer/module.go
  • apps/custom-issuer-go/modules/issuer/handler/issuer.go
  • apps/custom-issuer-go/modules/metrics/module.go
  • apps/custom-issuer-go/modules/health/module.go
📚 Learning: 2026-03-05T14:37:07.020Z
Learnt from: kpitapeersyst
Repo: Peersyst/fast-auth PR: 78
File: apps/custom-issuer-go/server.go:24-24
Timestamp: 2026-03-05T14:37:07.020Z
Learning: In `apps/custom-issuer-go/server.go`, `middleware.RecoveryMiddleware` is intentionally applied without any prior redaction middleware. The maintainer has confirmed that no sensitive/private data (e.g., Authorization header values, `jwt`, or `signPayload` body fields) will be logged in practice, so raw header and body logging in `RecoveryMiddleware` is acceptable for this service.

Applied to files:

  • apps/custom-issuer-go/main.go
📚 Learning: 2026-03-11T15:50:07.064Z
Learnt from: CR
Repo: Peersyst/agent-skills PR: 0
File: skills/go-mistakes-performance/AGENTS.md:0-0
Timestamp: 2026-03-11T15:50:07.064Z
Learning: Applies to skills/go-mistakes-performance/**/*_test.go : Use net/http/httptest for testing HTTP handlers and clients without real network calls, and testing/iotest for I/O interface testing

Applied to files:

  • apps/custom-issuer-go/modules/issuer/handler/issuer.go
📚 Learning: 2026-03-05T14:31:22.787Z
Learnt from: kpitapeersyst
Repo: Peersyst/fast-auth PR: 78
File: apps/custom-issuer-go/go.mod:1-1
Timestamp: 2026-03-05T14:31:22.787Z
Learning: In apps/custom-issuer-go/go.mod, the module path is intentionally declared as 'github.com/peersyst/fast-auth/apps/custom-issuer' (no '-go' suffix) while the directory is named 'custom-issuer-go'. Do not flag this directory-name/module-path mismatch as an issue; this is a temporary alias during transition and will be renamed to 'custom-issuer' after NestJS is removed.

Applied to files:

  • apps/custom-issuer-go/go.mod
🔇 Additional comments (13)
apps/custom-issuer-go/modules/issuer/service/service.go (1)

85-88: LGTM!

The HasValidationKeys() method correctly delegates to GetKeys(), which is already thread-safe (uses RLock per context snippet 2). The implementation is concise and appropriate for the health check use case.

apps/custom-issuer-go/modules/kms/service/service.go (1)

72-82: LGTM!

The Ping method correctly mirrors the production SignJWT signing pattern: it computes a SHA-256 digest and calls KMS Sign with MessageTypeDigest and the same RS256 algorithm. Discarding the signature and returning only the error is appropriate for a health probe.

apps/custom-issuer-go/main.go (1)

27-28: LGTM!

The module registration order correctly places health.Module after kms.Module and issuer.Module, which is required since the health module depends on services from both of those modules for its ValidationKeyChecker and SignProber interfaces.

apps/custom-issuer-go/modules/issuer/module.go (1)

21-21: LGTM!

Exporting the Service field enables cross-module access, which is required for the health module to retrieve the IssuerService and call HasValidationKeys(). This follows the same pattern used by the kms.Module.

Also applies to: 42-42, 63-63

apps/custom-issuer-go/modules/issuer/handler/issuer.go (1)

62-77: LGTM!

The metrics instrumentation is well-placed:

  • Duration is observed for all attempts (both success and failure), which provides visibility into total processing time regardless of outcome.
  • TokensFailedTotal is incremented on the error path before returning.
  • TokensIssuedTotal is incremented only after successful issuance.
apps/custom-issuer-go/modules/issuer/metrics/metrics.go (1)

21-23: Metric registration via init() is acceptable but has trade-offs.

Using init() with MustRegister is a common pattern. Note that it will panic at startup if metrics are registered twice (e.g., in tests that import this package multiple times). This is generally fine for production but worth being aware of for testing scenarios.

apps/custom-issuer-go/modules/health/handler/handler.go (2)

26-28: LGTM!

The /livez endpoint correctly implements a minimal liveness probe by returning only 200 OK with no body, which is appropriate for Kubernetes liveness checks.


30-36: LGTM!

The /readyz endpoint correctly maps the health service status to appropriate HTTP codes: 200 OK for healthy and 503 Service Unavailable for degraded states. The JSON response body provides additional detail for debugging.

apps/custom-issuer-go/go.mod (1)

9-9: Dependency version is current. github.com/prometheus/client_golang v1.23.2 is the latest release and appropriately added to support the new metrics functionality.

apps/custom-issuer-go/modules/metrics/module.go (1)

1-33: LGTM!

The metrics module is well-structured, follows the established AppModule pattern, and correctly exposes the Prometheus /metrics endpoint via promhttp.Handler(). The compile-time interface check at line 11 is a good practice.

apps/custom-issuer-go/modules/health/module.go (1)

1-59: LGTM!

The health module correctly retrieves and type-asserts the issuer and kms modules to access their services. The module initialization order in main.go ([kms, issuer, health, metrics]) combined with sequential initialization in NewAppModules guarantees that both dependencies are fully initialized before health.Init runs. Good error handling for both module retrieval and type assertion failures.

apps/custom-issuer-go/modules/health/service/service.go (2)

8-44: LGTM!

Clean type definitions with appropriate JSON tags. The interfaces (ValidationKeyChecker, SignProber) provide good abstraction for dependency injection, making the service easily testable. The constructor follows idiomatic Go patterns.


46-73: Well-implemented health check with proper timeout handling.

The CheckHealth implementation correctly:

  • Uses a bounded 3-second timeout for the KMS ping to prevent hanging health checks
  • Properly defers context cancellation to avoid leaks
  • Aggregates dependency statuses to compute overall health

One minor consideration: the error message from signProber.Ping is included directly in the response (line 59). This is generally acceptable for health endpoints used by operators, but verify this doesn't expose sensitive internal details in your environment.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 8, 2026
Copy link
Copy Markdown

@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.

🧹 Nitpick comments (1)
apps/custom-issuer-go/modules/health/service/service.go (1)

51-63: Accept a parent context in CheckHealth to respect caller cancellation and shutdown signals.

The current implementation uses context.Background(), which prevents request cancellation and shutdown signals from affecting the health check. For request-driven operations like health probes, the parent context should be propagated to properly handle timeouts and graceful shutdown.

Proposed refactor
-func (s *HealthService) CheckHealth() ReadyzResponse {
+func (s *HealthService) CheckHealth(parent context.Context) ReadyzResponse {
 	checks := map[string]DependencyResult{}
+	if parent == nil {
+		parent = context.Background()
+	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	ctx, cancel := context.WithTimeout(parent, 3*time.Second)
 	defer cancel()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/custom-issuer-go/modules/health/service/service.go` around lines 51 -
63, CheckHealth currently creates its own root context via context.Background()
which prevents caller cancellation; change the method signature to accept a
parent context (e.g., func (s *HealthService) CheckHealth(ctx context.Context)
ReadyzResponse), propagate that context into the timeout call (use ctx, cancel
:= context.WithTimeout(ctx, 3*time.Second)) and pass the incoming ctx to
signProber.Ping; update any callers of HealthService.CheckHealth to provide
their context so cancellations and shutdown signals are respected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/custom-issuer-go/modules/health/service/service.go`:
- Around line 51-63: CheckHealth currently creates its own root context via
context.Background() which prevents caller cancellation; change the method
signature to accept a parent context (e.g., func (s *HealthService)
CheckHealth(ctx context.Context) ReadyzResponse), propagate that context into
the timeout call (use ctx, cancel := context.WithTimeout(ctx, 3*time.Second))
and pass the incoming ctx to signProber.Ping; update any callers of
HealthService.CheckHealth to provide their context so cancellations and shutdown
signals are respected.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dfcea92d-f13d-4316-a8be-e5ee961584ec

📥 Commits

Reviewing files that changed from the base of the PR and between a915a8d and f92a10f.

📒 Files selected for processing (1)
  • apps/custom-issuer-go/modules/health/service/service.go
📜 Review details
⏰ 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). (2)
  • GitHub Check: integration-packages / Build
  • GitHub Check: build-base / Build
🔇 Additional comments (1)
apps/custom-issuer-go/modules/health/service/service.go (1)

73-81: Status aggregation logic is clear and correct.

The final reduction from per-dependency checks to overall status is straightforward and returns deterministic readiness semantics.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 8, 2026
Copy link
Copy Markdown

@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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@infra/chart/templates/custom-issuer-go.yaml`:
- Around line 5-8: The chart currently adds prometheus.io/port,
prometheus.io/scrape and prometheus.io/path annotations (using
customIssuerGo.service.targetPort) but provides no ServiceMonitor/PodMonitor or
Prometheus scrape configuration; add a ServiceMonitor CRD template that targets
the Service created by the custom-issuer-go chart (matching labels/selectors
used by the Service) and points to port {{
.Values.customIssuerGo.service.targetPort }} and path /metrics, or alternatively
update chart documentation/values to state that Prometheus must be configured to
honor prometheus.io annotations and provide an example Prometheus scrape config;
ensure the new template references the same labels/selectors and port name used
by the Service so the monitor will discover the endpoints.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 16aff788-2f5b-4214-8d41-e1c067339fd8

📥 Commits

Reviewing files that changed from the base of the PR and between f92a10f and 59bab6a.

📒 Files selected for processing (1)
  • infra/chart/templates/custom-issuer-go.yaml
📜 Review details
⏰ 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). (2)
  • GitHub Check: integration-packages / Build
  • GitHub Check: build-base / Build
🔇 Additional comments (1)
infra/chart/templates/custom-issuer-go.yaml (1)

59-72: Probe configuration aligns with health handlers.

Good split: livenessProbe on /livez and readinessProbe on /readyz matches the registered handlers and keeps dependency checks in readiness only.

Comment thread infra/chart/templates/custom-issuer-go.yaml
coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 9, 2026
@kpitapeersyst kpitapeersyst marked this pull request as draft April 9, 2026 07:55
Copy link
Copy Markdown

@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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/custom-issuer-go/modules/issuer/module.go (1)

36-43: ⚠️ Potential issue | 🟠 Major

Initialize metrics before constructing the issuer service.

If issuermetrics.Init() fails here, NewIssuerService(...) has already been created and its background refresh loop never gets stopped. Move metrics init ahead of service construction, or stop svc on the error path. ``

💡 Proposed fix
-	svc, err := service.NewIssuerService(cfg, kmsMod.Service)
-	if err != nil {
-		return err
-	}
-
 	if err := issuermetrics.Init(); err != nil {
 		return err
 	}
+
+	svc, err := service.NewIssuerService(cfg, kmsMod.Service)
+	if err != nil {
+		return err
+	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/custom-issuer-go/modules/issuer/module.go` around lines 36 - 43, Move
issuermetrics.Init() to run before creating the issuer service or ensure svc is
stopped if metrics init fails: either call issuermetrics.Init() prior to
invoking service.NewIssuerService(cfg, kmsMod.Service), or after creating svc,
on any error from issuermetrics.Init() call the service stop/close method on svc
(e.g., svc.Stop() or svc.Close() depending on the service API) before returning
the error so the background refresh loop started by NewIssuerService is cleaned
up.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@apps/custom-issuer-go/modules/issuer/module.go`:
- Around line 36-43: Move issuermetrics.Init() to run before creating the issuer
service or ensure svc is stopped if metrics init fails: either call
issuermetrics.Init() prior to invoking service.NewIssuerService(cfg,
kmsMod.Service), or after creating svc, on any error from issuermetrics.Init()
call the service stop/close method on svc (e.g., svc.Stop() or svc.Close()
depending on the service API) before returning the error so the background
refresh loop started by NewIssuerService is cleaned up.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cafc38e2-6537-4879-a95b-0ac0fff63240

📥 Commits

Reviewing files that changed from the base of the PR and between 59bab6a and 3c52424.

⛔ Files ignored due to path filters (1)
  • apps/custom-issuer-go/go.sum is excluded by !**/*.sum
📒 Files selected for processing (10)
  • apps/custom-issuer-go/go.mod
  • apps/custom-issuer-go/main.go
  • apps/custom-issuer-go/modules/common/modules/modules.go
  • apps/custom-issuer-go/modules/health/handler/handler.go
  • apps/custom-issuer-go/modules/health/service/service.go
  • apps/custom-issuer-go/modules/issuer/handler/issuer.go
  • apps/custom-issuer-go/modules/issuer/metrics/metrics.go
  • apps/custom-issuer-go/modules/issuer/module.go
  • apps/custom-issuer-go/modules/metrics/module.go
  • apps/custom-issuer-go/server.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/custom-issuer-go/modules/health/handler/handler.go
  • apps/custom-issuer-go/modules/health/service/service.go
📜 Review details
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-03-11T15:50:07.064Z
Learnt from: CR
Repo: Peersyst/agent-skills PR: 0
File: skills/go-mistakes-performance/AGENTS.md:0-0
Timestamp: 2026-03-11T15:50:07.064Z
Learning: Applies to skills/go-mistakes-performance/**/*_test.go : Use net/http/httptest for testing HTTP handlers and clients without real network calls, and testing/iotest for I/O interface testing

Applied to files:

  • apps/custom-issuer-go/modules/common/modules/modules.go
📚 Learning: 2026-03-05T14:37:07.020Z
Learnt from: kpitapeersyst
Repo: Peersyst/fast-auth PR: 78
File: apps/custom-issuer-go/server.go:24-24
Timestamp: 2026-03-05T14:37:07.020Z
Learning: In `apps/custom-issuer-go/server.go`, `middleware.RecoveryMiddleware` is intentionally applied without any prior redaction middleware. The maintainer has confirmed that no sensitive/private data (e.g., Authorization header values, `jwt`, or `signPayload` body fields) will be logged in practice, so raw header and body logging in `RecoveryMiddleware` is acceptable for this service.

Applied to files:

  • apps/custom-issuer-go/server.go
  • apps/custom-issuer-go/main.go
  • apps/custom-issuer-go/go.mod
📚 Learning: 2026-03-05T14:31:30.013Z
Learnt from: kpitapeersyst
Repo: Peersyst/fast-auth PR: 78
File: apps/custom-issuer-go/go.mod:1-1
Timestamp: 2026-03-05T14:31:30.013Z
Learning: In the Peersyst/fast-auth repository, the Go module at `apps/custom-issuer-go/go.mod` intentionally declares `module github.com/peersyst/fast-auth/apps/custom-issuer` (without the `-go` suffix). The directory is named `custom-issuer-go` temporarily while the NestJS version of the app still exists; it will be renamed to `custom-issuer` once NestJS is removed. Do not flag this module path / directory name mismatch as an issue.

Applied to files:

  • apps/custom-issuer-go/modules/issuer/module.go
  • apps/custom-issuer-go/main.go
  • apps/custom-issuer-go/modules/metrics/module.go
📚 Learning: 2026-03-05T14:31:22.787Z
Learnt from: kpitapeersyst
Repo: Peersyst/fast-auth PR: 78
File: apps/custom-issuer-go/go.mod:1-1
Timestamp: 2026-03-05T14:31:22.787Z
Learning: In apps/custom-issuer-go/go.mod, the module path is intentionally declared as 'github.com/peersyst/fast-auth/apps/custom-issuer' (no '-go' suffix) while the directory is named 'custom-issuer-go'. Do not flag this directory-name/module-path mismatch as an issue; this is a temporary alias during transition and will be renamed to 'custom-issuer' after NestJS is removed.

Applied to files:

  • apps/custom-issuer-go/go.mod
🔇 Additional comments (3)
apps/custom-issuer-go/main.go (1)

42-68: Good shutdown sequencing.

Waiting on done keeps the final "server stopped" log aligned with actual server and module teardown completion.

apps/custom-issuer-go/modules/issuer/handler/issuer.go (1)

67-70: The nil check is sufficient; empty payloads are already rejected during JSON decoding.

The bytearray.Array.UnmarshalJSON() method explicitly returns ErrEmpty when the input array is empty (line 8 of the bytearray implementation). This means an empty JSON array will fail the decoder.Decode(&req) call (lines 33–36) and be caught as an "invalid_body" error before reaching the nil check at line 67. The nil check correctly handles only the case where the field is omitted or explicitly null. The proposed fix using len(req.SignPayload) == 0 is unreachable because empty payloads cannot produce a non-nil value.

			> Likely an incorrect or invalid review comment.
apps/custom-issuer-go/go.mod (1)

3-3: Build tooling is compatible.

Container build tooling (docker/custom-issuer-go.Dockerfile) uses Go 1.26, which satisfies the go.mod requirement of 1.25.0 and is compatible with the stdlib patterns (Handle/HandleFunc with "GET /" syntax, errors.Join) used in this module.

@kpitapeersyst kpitapeersyst changed the title feat(custom-issuer-go): add health check and Prometheus metrics feat(custom-issuer-go): add health check and opentelemetry metrics Apr 9, 2026
@kpitapeersyst kpitapeersyst changed the title feat(custom-issuer-go): add health check and opentelemetry metrics feat(custom-issuer-go): add health endpoints and OpenTelemetry metrics Apr 9, 2026
@AdriaCarrera AdriaCarrera merged commit 49da878 into main Apr 9, 2026
13 checks passed
@AdriaCarrera AdriaCarrera deleted the feat/custom-issuer-go-health-metrics branch April 9, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants