Revert "Fix: Setup NewRelic based on config" #34
Conversation
WalkthroughRemoves the DisableNewRelic config flag, makes New Relic initialization unconditional, and drops the distributed tracer enablement option in SetupNewRelic. Updates go.mod with several indirect dependency additions and some removals. No public APIs added; one config field removed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Application
participant NR as NewRelic Setup
participant OTEL as OpenTelemetry Setup
rect rgb(240, 248, 255)
note over App: Startup
App->>NR: SetupNewRelic()
note over NR: Initialization attempted (errors not surfaced)
App->>OTEL: SetupNROpenTelemetry() (if applicable)
end
sequenceDiagram
autonumber
participant App as Application
participant NR as NewRelic Setup
opt Before (previous behavior)
App->>NR: SetupNewRelic() [only if not disabled]
NR-->>App: err handled/logged
end
opt After (current behavior)
App->>NR: SetupNewRelic() (always)
note over NR: No error handling here
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
rahulpr22
requested review from
ankurs and
svetha-cvl
and removed request for
svetha-cvl
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go.mod (1)
35-76: Remove unused indirect dependencies
go mod tidy -vflags the following as unused and they should be removed:
• github.com/bugsnag/bugsnag-go
• github.com/bugsnag/panicwrap
• github.com/cenkalti/backoff/v4
• github.com/go-kit/kit
• github.com/gofrs/uuid
• github.com/kardianos/osext
• github.com/klauspost/compress
• github.com/matttproud/golang_protobuf_extensions
• go.opentelemetry.io/otel/exporters/otlp/internal/retry
• google.golang.org/genproto
Also confirm whether imports forgithub.com/newrelic/csec-go-agentandgithub.com/smarty/assertionsare required; if not, remove them sogo mod tidycleans up accordingly. Finally, rerun your vulnerability scan using validmodule@versioninputs.
🧹 Nitpick comments (1)
core.go (1)
137-137: Minor: Inconsistent indentation.Extra leading space before
err :=on line 137. While this doesn't affect functionality, maintaining consistent indentation improves readability.Apply this diff to fix the indentation:
- err := SetupNROpenTelemetry( + err := SetupNROpenTelemetry(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
config/config.go(0 hunks)core.go(2 hunks)go.mod(2 hunks)initializers.go(0 hunks)
💤 Files with no reviewable changes (2)
- initializers.go
- config/config.go
🧰 Additional context used
🧬 Code graph analysis (1)
core.go (1)
initializers.go (2)
SetupNewRelic(46-65)SetupNROpenTelemetry(292-308)
| log.Error(context.Background(), "Error setting up NewRelic tracing", "error", err) | ||
| } | ||
| } | ||
| SetupNewRelic(nrName, c.config.NewRelicLicenseKey, c.config.NewRelicDistributedTracing) |
There was a problem hiding this comment.
Capture and handle the error from SetupNewRelic.
The function returns an error, but it's being ignored. If New Relic initialization fails (e.g., invalid configuration, network issues), the error will be silently swallowed, making debugging difficult.
Apply this diff to handle the error:
- SetupNewRelic(nrName, c.config.NewRelicLicenseKey, c.config.NewRelicDistributedTracing)
+ if err := SetupNewRelic(nrName, c.config.NewRelicLicenseKey, c.config.NewRelicDistributedTracing); err != nil {
+ log.Error(context.Background(), "msg", "Failed to setup New Relic", "err", err)
+ }🤖 Prompt for AI Agents
In core.go around line 97, the call to SetupNewRelic(nrName,
c.config.NewRelicLicenseKey, c.config.NewRelicDistributedTracing) ignores its
returned error; capture the return value (err := SetupNewRelic(...)) and handle
it by checking if err != nil then wrap and return the error (e.g. return
fmt.Errorf("failed to initialize New Relic: %w", err)) or log it with contextual
details and propagate the failure up so initialization doesn't silently
continue; add required imports (fmt or your logger) as needed.
| } else if c.config.NewRelicOpentelemetry { | ||
| // Fall back to New Relic OpenTelemetry if no custom OTLP is configured | ||
| err := SetupNROpenTelemetry( | ||
| err := SetupNROpenTelemetry( |
2 participants
Reverts #33
Summary by CodeRabbit
Refactor
Chores