Skip to content

feat: add event context #700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 27, 2025
Merged

Conversation

stympy
Copy link
Member

@stympy stympy commented Jun 26, 2025

Data added to event context will be added to all events reported to the API

@stympy stympy requested review from rabidpraxis and Copilot June 26, 2025 19:16
@stympy stympy linked an issue Jun 26, 2025 that may be closed by this pull request
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for setting and retrieving event-specific context on the Honeybadger agent, and ensures only that context is sent with events.

  • Delegates Honeybadger.event_context and Honeybadger.get_event_context to the agent singleton
  • Implements set_event_context/get_event_context in the context manager with global and block-local scopes
  • Updates Agent#event to include only event-specific context and the hostname when configured

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
spec/unit/honeybadger_spec.rb Adds tests for delegating module-level event_context and get_event_context
spec/unit/honeybadger/agent_spec.rb Adds comprehensive specs for event context behavior and event payload
lib/honeybadger/context_manager.rb Implements set_event_context, get_event_context, and reset logic
lib/honeybadger/agent.rb Adds public event_context/get_event_context API and payload updates
Comments suppressed due to low confidence (4)

spec/unit/honeybadger_spec.rb:50

  • It’s better to assert that event_context is called with the correct arguments. Consider updating this to .with(user_id: 123) to fully validate the delegation.
    expect(Honeybadger::Agent.instance).to receive(:event_context)

lib/honeybadger/context_manager.rb:57

  • [nitpick] The variable name local is ambiguous; consider renaming it to block_context or with_block to clarify its purpose.
      local = block_given?

lib/honeybadger/agent.rb:469

  • [nitpick] The example uses Honeybadger.event_context.clear!, but clear! lives on the agent, not on the return of event_context. Consider clarifying this or providing an explicit clear_event_context helper to avoid confusion.
    #   Honeybadger.event_context.clear!

lib/honeybadger/agent.rb:416

  • [nitpick] The indentation of this comment and the following lines in the tap block is inconsistent with the surrounding block; aligning them will improve readability.
        # Include only event-specific context in events (not global context)

Comment on lines +82 to +84
return @global_event_context unless @local_event_context

@global_event_context.merge(@local_event_context.inject({}, :merge))
Copy link
Preview

Copilot AI Jun 26, 2025

Choose a reason for hiding this comment

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

[nitpick] To prevent external code from mutating the internal state, consider returning a duplicate of the stored hash (e.g., @global_event_context.dup) rather than the original object.

Suggested change
return @global_event_context unless @local_event_context
@global_event_context.merge(@local_event_context.inject({}, :merge))
return @global_event_context.dup unless @local_event_context
@global_event_context.merge(@local_event_context.inject({}, :merge)).dup

Copilot uses AI. Check for mistakes.

Data added to event context will be added to all events reported to the API
@stympy stympy force-pushed the 699-feature-event-context-support-for-ruby branch from 3ba2bb2 to a389845 Compare June 26, 2025 21:24
stympy added 3 commits June 26, 2025 14:37
…d to fail

Hopefully this will fix the test failures for ruby-head, but if not, at least it should let the other tests continue.
The test "warns the logger when the queue has additional items" was failing
intermittently in JRuby because it expected exactly 1 warning but sometimes
received 2 warnings due to a race condition between the shutdown method and
work method both logging throttled warnings.
@stympy stympy requested a review from joshuap June 26, 2025 22:03
Copy link
Contributor

@rabidpraxis rabidpraxis left a comment

Choose a reason for hiding this comment

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

This looks good to me :)

@stympy stympy merged commit 36abc18 into master Jun 27, 2025
86 checks passed
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.

[Feature] Event Context support for Ruby
3 participants