refactor(middleware): migrate observability to lib-observability + 2.8.0-beta.1#99
Merged
Conversation
Replace logging and tracing imports from lib-commons with the dedicated lib-observability module in auth middlewares. This decouples the observability responsibilities from the general commons library to improve modularity and maintainability. Update Go version to 1.26.3 and upgrade dependencies, including grpc, opentelemetry, and lib-commons, to support the migration and keep the modules up to date.
## [2.8.0-beta.1](v2.7.0...v2.8.0-beta.1) (2026-05-18)
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR migrates observability instrumentation from lib-commons' OpenTelemetry helpers to lib-observability's tracing API across HTTP and gRPC middleware, updates test imports, bumps Go and selected dependencies, adjusts CI to Go 1.26 and gosec installation, and prepends a ChangesObservability Library Migration
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@auth/middleware/middleware.go`:
- Around line 424-427: The tracing call SetSpanAttributesFromValue is currently
sending the entire requestBody (used as "app.request.payload") which may include
OAuth credentials like clientSecret; before calling
tracing.SetSpanAttributesFromValue(span, "app.request.payload", requestBody,
nil) sanitize the payload by cloning or copying requestBody and
removing/redacting any sensitive keys (e.g., "clientSecret", "client_secret",
"password", "token") or replace their values with a fixed placeholder, then pass
the sanitized object to tracing.SetSpanAttributesFromValue; ensure this change
is applied where span and requestBody are used so no secret fields are sent to
telemetry.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 641c9aab-9990-4fb9-870a-1e96f38851b8
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
CHANGELOG.mdauth/middleware/middleware.goauth/middleware/middlewareGRPC.goauth/middleware/middleware_test.gogo.mod
go.mod targets Go 1.26.3 after the lib-observability migration. The pinned golangci-lint v2.4.0 was built with Go 1.25 and refuses to lint a 1.26-targeted module. The gosec Docker action ships Go 1.26.2 with GOTOOLCHAIN=local, so it also can't satisfy the module floor. Float golangci-lint to latest, bump the action's internal go_version to 1.26, and replace the gosec Docker action with a host-side go install so the setup-go pin actually controls gosec's runtime.
fredcamaral
temporarily deployed
to
create_release
May 18, 2026 20:12 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
## [2.8.0-beta.2](v2.8.0-beta.1...v2.8.0-beta.2) (2026-05-18)
|
🎉 This PR is included in version 2.8.0-beta.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/go-combined-analysis.yml:
- Line 56: Replace the unconstrained golangci_lint_version value of "latest"
with a specific pinned release (e.g., "v1.59.0") and likewise pin gosec by
adding or replacing gosec_version with an explicit version (e.g., "v2.14.0");
update any workflow steps or inputs that reference golangci_lint_version and
gosec_version so they consume these pinned variables to ensure reproducible
CI/security scans (look for the golangci_lint_version variable and any
gosec-related inputs in the workflow and set concrete semantic-version tags
instead of "latest").
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 53411899-b79f-4379-be7a-7c58f6b9b98b
📒 Files selected for processing (2)
.github/workflows/go-combined-analysis.ymlCHANGELOG.md
…b-observability Replace remaining four call sites still using the deprecated commons.NewTrackingFromContext with the lib-observability equivalent. Tail of b32322c, which migrated the bulk of the observability layer but missed these middleware call sites and triggered staticcheck SA1019 findings on PR #99.
fredcamaral
temporarily deployed
to
create_release
May 18, 2026 20:17 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
## [2.8.0-beta.3](v2.8.0-beta.2...v2.8.0-beta.3) (2026-05-18)
|
🎉 This PR is included in version 2.8.0-beta.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
auth/middleware/middleware.go (1)
419-426:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRedact OAuth secret from telemetry payload.
Line 425 sends
requestBodyto span attributes, andrequestBodyincludesclientSecret(Line 422). This can leak credentials into tracing backends.🔒 Proposed fix
requestBody := map[string]string{ "grantType": "client_credentials", "clientId": clientID, "clientSecret": clientSecret, } - err := tracing.SetSpanAttributesFromValue(span, "app.request.payload", requestBody, nil) + tracePayload := map[string]string{ + "grantType": "client_credentials", + "clientId": clientID, + } + + err := tracing.SetSpanAttributesFromValue(span, "app.request.payload", tracePayload, nil) if err != nil { tracing.HandleSpanError(span, "Failed to convert request body to JSON string", err) return "", fmt.Errorf("failed to convert request body to JSON string: %w", err) }#!/bin/bash set -euo pipefail # Verify no sensitive credential keys are attached to tracing payload attributes. rg -n -C4 'SetSpanAttributesFromValue\(.+app\.request\.payload' auth/middleware/middleware.go auth/middleware/middlewareGRPC.go rg -n -C4 '"clientSecret"|"client_secret"|"password"|"secret"|"accessToken"|"refreshToken"' auth/middleware/middleware.go auth/middleware/middlewareGRPC.go🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@auth/middleware/middleware.go` around lines 419 - 426, The tracing payload currently includes sensitive credentials: the map requestBody (used in tracing.SetSpanAttributesFromValue with span) contains clientSecret; change the code to exclude or redact the secret before attaching to tracing (e.g., create a copy of requestBody or a sanitized map and either remove the "clientSecret" key or replace its value with a mask like "[REDACTED]"), then pass that sanitized map to tracing.SetSpanAttributesFromValue(span, "app.request.payload", sanitizedRequestBody, nil); also audit any other uses of tracing.SetSpanAttributesFromValue in middlewareGRPC.go to ensure no secret keys (client_secret, password, accessToken, refreshToken) are sent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 1-6: CHANGELOG shows empty entries for releases 2.8.0-beta.1,
2.8.0-beta.2 and 2.8.0-beta.3; regenerate or restore the missing changelog
content by re-running your changelog generation (semantic-release /
conventional-changelog) so the commits `refactor(middleware): migrate
observability to lib-observability package`, `ci: bump go-version to 1.26 and
golangci-lint to latest`, and `refactor(middleware): complete
NewTrackingFromContext migration to lib-observability` are included under the
respective 2.8.0-beta.* headings, and verify .releaserc.yml still includes the
'refactor' and 'ci' types and that the changelog plugin executed successfully
(inspect the release automation logs and re-run the release step if necessary).
---
Duplicate comments:
In `@auth/middleware/middleware.go`:
- Around line 419-426: The tracing payload currently includes sensitive
credentials: the map requestBody (used in tracing.SetSpanAttributesFromValue
with span) contains clientSecret; change the code to exclude or redact the
secret before attaching to tracing (e.g., create a copy of requestBody or a
sanitized map and either remove the "clientSecret" key or replace its value with
a mask like "[REDACTED]"), then pass that sanitized map to
tracing.SetSpanAttributesFromValue(span, "app.request.payload",
sanitizedRequestBody, nil); also audit any other uses of
tracing.SetSpanAttributesFromValue in middlewareGRPC.go to ensure no secret keys
(client_secret, password, accessToken, refreshToken) are sent.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 43d2ac82-4a9b-4b77-b093-0809a5f42066
📒 Files selected for processing (3)
CHANGELOG.mdauth/middleware/middleware.goauth/middleware/middlewareGRPC.go
qnen
approved these changes
May 18, 2026
GetApplicationToken was passing the OAuth client_credentials request body — including clientSecret — to tracing.SetSpanAttributesFromValue, leaking the long-lived service credential into every consumer's observability pipeline (OTel collector, Datadog, Jaeger, etc.). Introduce a separate tracePayload map at the call site that mirrors the wire payload but omits clientSecret. The HTTP request body (json.Marshal of the unchanged requestBody) is unaffected, so authentication behavior is identical. Inline comment warns against collapsing the two maps in any future DRY-up.
fredcamaral
temporarily deployed
to
create_release
May 18, 2026 20:56 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
## [2.8.0-beta.4](v2.8.0-beta.3...v2.8.0-beta.4) (2026-05-18) ### Bug Fixes * **security:** redact clientSecret from tracing span payload ([fb15a94](fb15a94))
|
🎉 This PR is included in version 2.8.0-beta.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Member
Author
CodeRabbit Autofix Review CompleteReviewed 3 CodeRabbit feedback items. Applied (1):
Deferred (2):
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
CHANGELOG.md (1)
8-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPopulate the empty beta release sections (
2.8.0-beta.3/.2/.1).These headings are still empty, so the changelog does not document what changed in those beta cuts. Please regenerate/restore those entries so each beta release has its corresponding notes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` around lines 8 - 13, The three beta release headings in CHANGELOG.md (2.8.0-beta.3, 2.8.0-beta.2, 2.8.0-beta.1) are empty; regenerate or restore their release notes so each heading lists the actual changes — for each heading (2.8.0-beta.3 / 2.8.0-beta.2 / 2.8.0-beta.1) restore the corresponding release notes by re-running your changelog generator (or copying the released notes/PR summaries and commit links from the repository or release pages) and paste them under the appropriate heading, including short descriptions, PR/commit references and dates so each beta entry documents its changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@CHANGELOG.md`:
- Around line 8-13: The three beta release headings in CHANGELOG.md
(2.8.0-beta.3, 2.8.0-beta.2, 2.8.0-beta.1) are empty; regenerate or restore
their release notes so each heading lists the actual changes — for each heading
(2.8.0-beta.3 / 2.8.0-beta.2 / 2.8.0-beta.1) restore the corresponding release
notes by re-running your changelog generator (or copying the released notes/PR
summaries and commit links from the repository or release pages) and paste them
under the appropriate heading, including short descriptions, PR/commit
references and dates so each beta entry documents its changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fbeb7b6b-d417-4195-8ae3-7f30a470b9a2
📒 Files selected for processing (2)
CHANGELOG.mdauth/middleware/middleware.go
gandalf-at-lerian
approved these changes
May 18, 2026
gandalf-at-lerian
left a comment
gandalf-at-lerian
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the observability migration and the CodeRabbit follow-up.
No blocking findings from my side. I verified the PR at e1a25d9 with:
go test ./...go build ./...- GitHub workflow
Go Combined Analysisis green on the head commit
One release note before stable: NewAuthClient now takes *github.com/LerianStudio/lib-observability/log.Logger, so downstream consumers passing a custom logger from lib-commons/commons/log need to migrate imports. That is fine for this beta, but the smoke test should explicitly cover that path before promoting.
Member
Author
|
@CodeRabbit approve |
✅ Actions performedComments resolved and changes approved. |
Add TestGetApplicationToken_DoesNotTraceClientSecret to ensure that the client secret is not included in OpenTelemetry span attributes. This prevents sensitive credentials from leaking into observability tools during application token generation.
fredcamaral
temporarily deployed
to
create_release
May 18, 2026 21:18 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
|
🎉 This PR is included in version 2.8.0-beta.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.8.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
auth/middlewareobservability from in-tree wiring to the sharedlib-observabilitypackage (HTTP + gRPC middlewares).2.8.0-beta.1(CHANGELOG updated).go.mod/go.sumto align with the new dependency surface.Test plan
go build ./...greengo test ./...green (includingauth/middleware/middleware_test.go)2.8.0-beta.1tag before promoting to stable