Skip to content
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

feat: http(s) backend implementation for events #520

Merged
merged 14 commits into from
Feb 7, 2024

Conversation

halfbyte
Copy link
Contributor

I've added a hopefully sufficient amount of tests and also tested this manually against the actual API

halfbyte and others added 14 commits December 8, 2023 18:30
This currently is missing a queue and calls the backend directly from the agent.

Should I implement an events_worker within this PR or in the PR that adds the server backend?
Given that the worker relies on the Queue as the main scheduling mechanism I saw no other way than to
start a second thread that occasionally throws a message into the queue to check if the timeout is reached.

This seems to work in testing.
This adds a minimal set of tests to ensure API conformance

I've tested the code manually against "the real thing(tm)"
@halfbyte halfbyte requested review from joshuap and subzero10 January 25, 2024 11:30
Copy link
Member

@subzero10 subzero10 left a comment

Choose a reason for hiding this comment

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

LGTM, but I will let @joshuap or @halfbyte approve!

Copy link
Member

@joshuap joshuap left a comment

Choose a reason for hiding this comment

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

Looks good @halfbyte, thanks!

@joshuap joshuap merged commit 637e192 into feat-event-method Feb 7, 2024
@joshuap joshuap deleted the feat-event-http-backend branch February 7, 2024 21:02
joshuap added a commit that referenced this pull request Feb 12, 2024
* Implements Honeybadger.event by sync log call

* API changes, add timestamp

* fix #event: Use Hash() and reverse merge order.

* feat: implement simple debug backend endpoint for events (#513)

* Implement simple debug backend endpoint for events

This currently is missing a queue and calls the backend directly from the agent.

Should I implement an events_worker within this PR or in the PR that adds the server backend?

* Refactor signature of events backend to take only one argument

* WIP: Add worker

* WIP start of worker spec

* Worker spec successfully duplicated

* Implement timeout mechanism using separate thread

Given that the worker relies on the Queue as the main scheduling mechanism I saw no other way than to
start a second thread that occasionally throws a message into the queue to check if the timeout is reached.

This seems to work in testing.

* Remove one timeout check, namespace config

* Remove unused code

* Add events worker to agent stop/flush commands

* Fix debug message in events worker

---------

Co-authored-by: Joshua Wood <[email protected]>

* Slightly bump sleep values in test to fix jruby tests

There seems to be a slight difference in how sleep works in jruby so the timeouts in the tests did not hit predictably.

* install sqlite dev package for rails tests

* use sudo

* Okay, sqlite problem seems to be based on rubygems issue

sparklemotion/sqlite3-ruby#411

* I have no idea what I'm doing

* feat: http(s) backend implementation for events (#520)

* Implement simple debug backend endpoint for events

This currently is missing a queue and calls the backend directly from the agent.

Should I implement an events_worker within this PR or in the PR that adds the server backend?

* Refactor signature of events backend to take only one argument

* WIP: Add worker

* WIP start of worker spec

* Worker spec successfully duplicated

* Implement timeout mechanism using separate thread

Given that the worker relies on the Queue as the main scheduling mechanism I saw no other way than to
start a second thread that occasionally throws a message into the queue to check if the timeout is reached.

This seems to work in testing.

* Remove one timeout check, namespace config

* Remove unused code

* Add server back end functionality for events

This adds a minimal set of tests to ensure API conformance

I've tested the code manually against "the real thing(tm)"

* Add events worker to agent stop/flush commands

* Fix debug message in events worker

---------

Co-authored-by: Joshua Wood <[email protected]>

* Support Hash as first argument to Honeybadger#event (#521)

This enables both signatures:

   # With event type as first argument (recommended):
   Honeybadger.event("user_signed_up", user_id: 123)

   # With just a payload:
   Honeybadger.event(event_type: "user_signed_up", user_id: 123)

* Don't memoize events config

The config is initialized after the agent is created (when the app loads).

* Lazy initialize events worker

This results in less change for current users—if you aren't using
insights, the extra threads don't need to run. We could change this back
in the future.

---------

Co-authored-by: Joshua Wood <[email protected]>
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.

3 participants