Skip to content

Comments

feat: new relic integration#3077

Open
yotor1221 wants to merge 6 commits intosuperplanehq:mainfrom
yotor1221:integration/newrelic
Open

feat: new relic integration#3077
yotor1221 wants to merge 6 commits intosuperplanehq:mainfrom
yotor1221:integration/newrelic

Conversation

@yotor1221
Copy link

@yotor1221 yotor1221 commented Feb 12, 2026

feat: Add New Relic integration OnIssue ,runNRQL and reportmetric

Description

Implements #2552.

This PR implements the native New Relic integration, which allows users to trigger workflows based on New Relic issues (OnIssue), execute deep telemetry queries (RunNRQLQuery), and report custom business metrics (ReportMetric) back to New Relic.

Authorization is via API keys:

  • RunNRQLQuery: Requires a User API Key (starts with NRAK-).
  • ReportMetric: Recommended to use with a License Key (Ingest - License).

a video demo

https://www.loom.com/share/b5554aa5781e49e98b55eb0bab80a9da

Backend Implementation

The backend implementation code is in pkg/integrations/newrelic/.

  • Structure: Follows existing patterns using newrelic.go for registration and separate files for triggers/actions (on_issue.go, run_nrql_query.go, report_metric.go).
  • Key Features:
    • GPG Signing: All commits are GPG signed.
    • Configuration Persistence: Fixed UI state issues by enforcing strict string typing in OnIssue and RunNRQLQuery structs.
    • Smart Templates: Added logic to intercept unresolved template tags (e.g., {{account_id}}) to prevent invalid API calls.
  • Example Output: JSON examples for component outputs are included in the code comments and documentation.

Frontend Implementation

This integration uses the Server-Driven UI (SDUI) pattern. The frontend forms are automatically generated from the Go configuration structs defined in the backend.

  • Persistence Fix: ensuring field types (e.g., FieldTypeIntegrationResource) map correctly to string inputs in the UI has resolved previous selection issues.

Docs

Documentation has been generated in docs/components/New Relic.mdx by running make gen.components.docs.

It includes:

  • Setup instructions (API Key generation).
  • Usage guides for OnIssue, Run NRQL Query, and Report Metric.
  • Example configuration and data payloads.

Tests

Unit tests are located in pkg/integrations/newrelic/.

  • run_nrql_query_test.go: Covers query execution, template variable validation, and context fallback logic.
  • on_issue_test.go: Covers webhook validation, payload parsing, and event filtering.
  • repro_test.go: Specifically tests UI configuration persistence and type handling.
  • report_metric_test.go: Covers metric payload construction.

Tests are deterministic and pass locally.

@AleksandarCole AleksandarCole added pr:stage-1/3 Needs to pass basic review. wfh labels Feb 12, 2026
@yotor1221 yotor1221 force-pushed the integration/newrelic branch from a515431 to 9a0df7d Compare February 12, 2026 19:42
@yotor1221 yotor1221 force-pushed the integration/newrelic branch from f715ec1 to d3d3746 Compare February 13, 2026 09:02
@yotor1221 yotor1221 force-pushed the integration/newrelic branch 4 times, most recently from a4ab7c0 to 14bdcfc Compare February 13, 2026 12:59
@yotor1221 yotor1221 force-pushed the integration/newrelic branch 2 times, most recently from 7e664c5 to 7fd75d9 Compare February 13, 2026 14:59
@yotor1221 yotor1221 force-pushed the integration/newrelic branch 7 times, most recently from eda61d7 to 17d7d89 Compare February 14, 2026 04:36
@yotor1221 yotor1221 force-pushed the integration/newrelic branch from 17d7d89 to de18403 Compare February 14, 2026 04:55
@yotor1221 yotor1221 force-pushed the integration/newrelic branch from de18403 to 239ed05 Compare February 14, 2026 05:23
@yotor1221 yotor1221 force-pushed the integration/newrelic branch 2 times, most recently from 43e059a to ccd8a72 Compare February 18, 2026 14:46
@yotor1221 yotor1221 force-pushed the integration/newrelic branch from ccd8a72 to dedc9cd Compare February 18, 2026 15:22
@yotor1221 yotor1221 force-pushed the integration/newrelic branch from dedc9cd to 531d7d3 Compare February 18, 2026 15:56
@yotor1221 yotor1221 force-pushed the integration/newrelic branch from 531d7d3 to acc4a90 Compare February 18, 2026 19:09
return fmt.Errorf("async query timed out: retry deadline exceeded")
}

return ctx.Requests.ScheduleActionCall("poll", map[string]any{}, PollInterval)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsed RetryAfter field never used for poll scheduling

Medium Severity

The QueryProgress.RetryAfter field is defined, parsed from API responses, and asserted in tests, but never used when scheduling the next poll. Both the initial poll (in Execute) and subsequent polls (in poll) always use the fixed PollInterval of 10 seconds. Meanwhile, RetryDeadline from the same struct IS used for timeout detection. If New Relic recommends a shorter or longer wait via retryAfter, the code ignores it — potentially over-polling (risking rate limits) or under-polling (adding unnecessary latency).

Additional Locations (1)

Fix in Cursor Fix in Web

@yotor1221 yotor1221 force-pushed the integration/newrelic branch from acc4a90 to 757b6e5 Compare February 19, 2026 06:03
config := OnIssueConfiguration{}
if err := mapstructure.Decode(ctx.Configuration, &config); err != nil {
return fmt.Errorf("failed to decode configuration: %w", err)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused decoded config in OnIssue Setup method

Low Severity

The config variable is decoded from ctx.Configuration in step 3 of Setup, but its values (Priorities, States) are never accessed afterward. The only effect is a decode error return if ctx.Configuration is structurally incompatible, which for a []string slice-only struct would never happen in practice. Every other integration in the codebase that decodes config in Setup actively uses the decoded values for validation or API calls; integrations that don't need config simply return nil without decoding. This is dead code.

Fix in Cursor Fix in Web

db/structure.sql Outdated
-- Name: idx_casbin_rule; Type: INDEX; Schema: public; Owner: -
--

CREATE UNIQUE INDEX idx_casbin_rule ON public.casbin_rule USING btree (ptype, v0, v1, v2, v3, v4, v5);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unique index ineffective on nullable casbin columns

Medium Severity

The new idx_casbin_rule unique index covers all seven nullable columns (ptype, v0v5). In PostgreSQL, B-tree unique indexes treat NULL as distinct from every other NULL, so rows that share identical non-null values but have NULL in any indexed column (e.g., v3v5 for typical 4-field policies) are not considered duplicates. Additionally, ptype was changed from NOT NULL to nullable in this same diff, further weakening the constraint. The original migration (20250605035651_add-casbin-rbac-rules.up.sql) created v0v5 as nullable, so existing data may already contain NULL values in those columns. The gorm-adapter v3 relies on this composite unique constraint to prevent duplicate policy entries, but for the most common casbin policy shape (3–4 fields with NULL remainder), the index provides no duplicate protection.

Additional Locations (1)

Fix in Cursor Fix in Web

@yotor1221 yotor1221 force-pushed the integration/newrelic branch from 757b6e5 to 913178b Compare February 19, 2026 06:29
interval := PollInterval
if response.QueryProgress != nil && response.QueryProgress.RetryAfter > 0 {
interval = time.Duration(response.QueryProgress.RetryAfter) * time.Second
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant null check after non-null invariant is guaranteed

Low Severity

In Execute, the guard at lines 269–271 returns early if response.QueryProgress == nil, so at line 283, response.QueryProgress is always non-nil. The repeated response.QueryProgress != nil check is dead code. The same pattern appears in poll at line 344, inside the body of if response.QueryProgress != nil && !response.QueryProgress.Completed. These redundant checks misrepresent which invariants hold at those points and could mislead future maintainers into believing a nil code path exists (or accidentally mask a nil dereference if the guard above is ever removed).

Additional Locations (1)

Fix in Cursor Fix in Web

@yotor1221 yotor1221 force-pushed the integration/newrelic branch 3 times, most recently from 23783a0 to fd8c0e4 Compare February 19, 2026 07:30
core.DefaultOutputChannel.Name,
RunNRQLQueryPayloadType,
[]any{payload},
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Poll duplicates emission logic from emitResults

Low Severity

The poll method (lines 352–365) contains an identical copy of the payload construction and ExecutionState.Emit call that already exists in emitResults (lines 377–391). The duplication exists because emitResults accepts core.ExecutionContext while poll receives core.ActionContext. Any future change to the output shape (e.g., adding a field to RunNRQLQueryPayload or changing the channel name) must be applied in both places, making divergence between synchronous and asynchronous query results likely.

Additional Locations (1)

Fix in Cursor Fix in Web

@yotor1221 yotor1221 force-pushed the integration/newrelic branch from fd8c0e4 to 5a5613e Compare February 19, 2026 07:55
"type": metric.Type,
"timestamp": metric.Timestamp,
"intervalMs": metric.IntervalMs,
"status": "202 Accepted",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded output status ignores actual API response

Low Severity

The Execute function always emits "status": "202 Accepted" regardless of what the New Relic Metric API actually returned. client.ReportMetric accepts any 2xx response as success (200–299) but discards the actual status code, making the hardcoded value a fabrication. Other integrations in the codebase (e.g., SendGrid) use the real HTTP status from the response. If the API ever returns 200 or 201, the output silently misreports it.

Fix in Cursor Fix in Web

// emitResults emits the NRQL query results to the default output channel.
func (c *RunNRQLQuery) emitResults(ctx core.ExecutionContext, response *NRQLQueryResponse, query, accountIDStr string) error {
return c.emitPayload(ctx.ExecutionState, response, query, accountIDStr)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thin wrapper function adds unnecessary indirection

Low Severity

emitResults is a one-line wrapper whose sole purpose is to call emitPayload(ctx.ExecutionState, ...). It is called from exactly one place (Execute), while poll already calls emitPayload directly. Every other integration component in the codebase calls state.Emit (or ctx.ExecutionState.Emit) directly without an intermediate wrapper. The abstraction adds indirection without adding value.

Fix in Cursor Fix in Web

@yotor1221 yotor1221 force-pushed the integration/newrelic branch from 5a5613e to 78f5af3 Compare February 19, 2026 08:35
@yotor1221
Copy link
Author

Hi @lucaspin

I've updated the NRQL query logic to support asynchronous execution as suggested.

Fixed Timeout: Set a 10s timeout for the initial request.

Async Fallback: If the query doesn't complete within that window, the integration now captures the New Relic queryId.

Non-blocking Polling: I implemented ctx.Requests.ScheduleActionCall() to poll for the results. This ensures the execution doesn't hang and follows the project's async pattern.

Configuration: Removed the user-configurable timeout to keep the implementation clean and predictable.

The code passed all the tests . Ready for another look

}

// Validate and fetch accounts only when a User API Key is provided
if config.UserAPIKey != "" {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sync branches on encrypted config instead of decrypted values

Medium Severity

The Sync method reads config.UserAPIKey via mapstructure.Decode(ctx.Configuration, &config) and uses it for branching (if config.UserAPIKey != ""). However, ctx.Configuration contains encrypted values for sensitive fields (both userApiKey and licenseKey are Sensitive: true), while NewClient reads them via ctx.Integration.GetConfig() which decrypts and trims. This means config.UserAPIKey is an encrypted blob (always non-empty when stored), not the actual key. The branching decision could incorrectly enter the NerdGraph validation path. The correct pattern (used by Discord's Sync) is to use ctx.Integration.GetConfig() or client.UserAPIKey from the already-created client for the branching decision.

Fix in Cursor Fix in Web

@yotor1221 yotor1221 force-pushed the integration/newrelic branch from 1db7587 to 3a2b34b Compare February 20, 2026 06:55
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

return http.StatusInternalServerError, fmt.Errorf("failed to emit event: %w", err)
}

return http.StatusOK, nil
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Webhook handler missing authentication verification

High Severity

The OnIssue.HandleWebhook does not verify that incoming requests are authenticated. Every other trigger in the codebase (GitHub, Bitbucket, CircleCI, Prometheus, Grafana) authenticates webhook requests — using ctx.Webhook.GetSecret() with HMAC signature verification or shared-secret header checks. This handler processes the payload and emits events without any authentication, allowing anyone who discovers the webhook URL to trigger arbitrary workflow executions. This violates the project's security rule requiring HandleWebhook() to always verify requests using the webhook secret.

Fix in Cursor Fix in Web

Signed-off-by: Teshome Birhanu <[email protected]>
@yotor1221 yotor1221 force-pushed the integration/newrelic branch from 97ad55e to 07425b6 Compare February 20, 2026 08:07
@yotor1221
Copy link
Author

@lucaspin
I've updated the New Relic integration. Here’s what’s included:

Security: Implemented webhook authentication using ctx.Webhook.GetSecret() with constant-time comparison to prevent timing attacks.

Client Fix: Resolved the issue with encrypted credentials in the New Relic client logic to ensure proper decryption and API connectivity.

Docs: Regenerated the component documentation to include the new X-Superplane-Secret header requirement.

Tests: Added new test cases for authenticated/unauthenticated requests (all local tests passing).

The DCO is now signed, and I've synced the latest changes from main. Ready for final look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:stage-3/3 Ready for full, in-depth, review wfh

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants