Skip to content

feat: add scope merging mechanism - #1373

Open
giortzisg wants to merge 2 commits into
scopes/scope-storagefrom
scopes/scope-merge
Open

feat: add scope merging mechanism#1373
giortzisg wants to merge 2 commits into
scopes/scope-storagefrom
scopes/scope-merge

Conversation

@giortzisg

@giortzisg giortzisg commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Description

Issues

Changelog Entry Instructions

To add a custom changelog entry, uncomment the section above. Supports:

  • Single entry: just write text
  • Multiple entries: use bullet points
  • Nested bullets: indent 4+ spaces

For more details: custom changelog entries

Reminders

@linear-code

linear-code Bot commented Jul 23, 2026

Copy link
Copy Markdown

GO-155

@giortzisg
giortzisg force-pushed the scopes/scope-merge branch from 8f6a8b4 to e509cb6 Compare July 23, 2026 08:45
@giortzisg
giortzisg force-pushed the scopes/scope-storage branch from 0bb8a70 to 8ce983c Compare July 23, 2026 08:45
@giortzisg
giortzisg marked this pull request as ready for review July 23, 2026 08:51
Comment thread scope.go Outdated
@giortzisg
giortzisg force-pushed the scopes/scope-merge branch from e509cb6 to 5ea0d02 Compare July 30, 2026 20:45
@giortzisg
giortzisg force-pushed the scopes/scope-storage branch 2 times, most recently from 3ce9717 to 24cbd32 Compare August 4, 2026 15:02
@giortzisg
giortzisg force-pushed the scopes/scope-merge branch from 5ea0d02 to 2905cce Compare August 4, 2026 15:25
Comment thread scope.go Outdated
@giortzisg
giortzisg force-pushed the scopes/scope-merge branch 2 times, most recently from 8b46a73 to b3bd79f Compare August 5, 2026 09:59
@giortzisg
giortzisg force-pushed the scopes/scope-storage branch from 1ba25e1 to 5ebf737 Compare August 10, 2026 11:28
Comment thread scope.go Outdated
@giortzisg
giortzisg force-pushed the scopes/scope-storage branch 2 times, most recently from 14a0bf8 to 01f70b4 Compare August 10, 2026 13:23
@giortzisg
giortzisg force-pushed the scopes/scope-merge branch 2 times, most recently from a32be8d to af060b8 Compare August 12, 2026 12:11
Comment thread scope.go
Comment thread scope.go
@giortzisg
giortzisg force-pushed the scopes/scope-storage branch 2 times, most recently from a32c046 to 7c286e0 Compare August 13, 2026 11:48
Comment thread scope.go
Comment thread scope.go
Comment thread scope.go
Comment thread metrics.go
Comment on lines +58 to +59
options := client.options
if !options.DisableMetrics {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The NewMeter and NewLogger functions no longer return no-op instances when the client is disabled, leading to unnecessary work.
Severity: MEDIUM

Suggested Fix

Reinstate the client.IsEnabled() check in the conditional logic for NewMeter in metrics.go and NewLogger in log.go. This will ensure that a no-op instance is returned when the client is disabled, preserving the original behavior and matching the documentation.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: metrics.go#L58-L59

Potential issue: The `client.IsEnabled()` check was removed from the `NewMeter` and
`NewLogger` factory functions. As a result, if the Sentry SDK is not initialized (e.g.,
no DSN is configured), these functions will now return real `sentryMeter` and
`sentryLogger` instances instead of the intended no-op versions. This causes unnecessary
CPU and memory usage to build metrics and logs that are ultimately discarded, and it
creates a mismatch between the code's behavior and its documentation, which states that
a no-op instance should be returned when the client is disabled.

Also affects:

  • log.go:63~64

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f1ba13c. Configure here.

Comment thread client.go
@giortzisg
giortzisg force-pushed the scopes/scope-storage branch from 7c286e0 to e747b11 Compare August 13, 2026 12:51
@giortzisg
giortzisg force-pushed the scopes/scope-storage branch from e747b11 to 38e8186 Compare August 24, 2026 10:16
Comment thread scope.go
Comment on lines +746 to +756
}
if event.Contexts == nil {
event.Contexts = make(map[string]Context)
}
if _, ok := event.Contexts["trace"]; !ok {
traceID, spanID := any(trace.traceID), any(trace.spanID)
if trace.external {
traceID, spanID = trace.traceID.String(), trace.spanID.String()
}
event.Contexts["trace"] = Context{"trace_id": traceID, "span_id": spanID}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The applyTraceToEvent function only includes trace_id and span_id in the event's trace context, losing other important span metadata like op, description, and status.
Severity: HIGH

Suggested Fix

Restore the previous behavior. Instead of creating a new minimal trace context, use the full context from the active span. Modify applyTraceToEvent or its calling logic to check if a span exists on the scope and, if so, use scope.span.traceContext().Map() to populate the event.Contexts["trace"] field, ensuring all span metadata is preserved.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: scope.go#L743-L756

Potential issue: When an error event is captured with an active span in the scope, the
new `applyTraceToEvent` function creates a minimal trace context containing only
`trace_id` and `span_id`. The previous implementation copied the full trace context from
the span, including `parent_span_id`, `op`, `description`, `status`, and `data`. This
change results in the loss of this critical metadata, which will break the linking of
errors to their parent transactions and operations. The code explicitly skips merging
the "trace" context from the scope at `scope.go:430`.

Also affects:

  • scope.go:430~432

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.

1 participant