Skip to content

Add basic Honeycomb instrumentation#1921

Merged
njbennett merged 1 commit intomasterfrom
honeeycomb-pr
Oct 30, 2020
Merged

Add basic Honeycomb instrumentation#1921
njbennett merged 1 commit intomasterfrom
honeeycomb-pr

Conversation

@njbennett
Copy link
Copy Markdown
Contributor

When honeycomb.write_key is present, emit events for every incoming
HTTP request, except to the /healthz endpoint.

Authored-by: Nat Bennett nbennett@pivotal.io

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Oct 21, 2020

CLA Not Signed

@cf-gitbot
Copy link
Copy Markdown

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/175388131

The labels on this github issue will be updated when the story is started.

Comment thread config/initializers/honeycomb.rb
@tcdowney
Copy link
Copy Markdown
Member

Hmm, it looks like the postgres tests are failing with errors like this:

An error occurred while loading ./spec/acceptance/async_bindings_spec.rb.
Failure/Error: CCInitializers.send(method, @config_hash)

@matt-royal
Copy link
Copy Markdown
Contributor

It looks like the honeycomb-beeline gem is Apache 2 licensed, so there shouldn't be any licensing issues with adding this gem.

@njbennett
Copy link
Copy Markdown
Contributor Author

Force pushed some changes to the commit. Now we check only for the top-level honeycomb key, rather than a sub-key, because the call to get the subkey value with [] fails when the top level key returns nil.

use CloudFoundry::Middleware::Cors, config.get(:allowed_cors_domains)
use CloudFoundry::Middleware::VcapRequestId
use CloudFoundry::Middleware::NewRelicCustomAttributes if config.get(:newrelic_enabled)
use Honeycomb::Rack::Middleware, client: Honeycomb.client if config.get(:honeycomb, :write_key)
Copy link
Copy Markdown
Contributor

@cwlbraa cwlbraa Oct 29, 2020

Choose a reason for hiding this comment

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

should this also be conditional on config.get(:honeycomb)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. Updated.

When honeycomb.write_key is present, emit events for every incoming
HTTP request, except to the /healthz endpoint

Authored-by: Nat Bennett <nbennett@pivotal.io>
Copy link
Copy Markdown
Member

@tcdowney tcdowney left a comment

Choose a reason for hiding this comment

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

LGTM apart from the CLA bit 🤔

@njbennett njbennett merged commit 95f7465 into master Oct 30, 2020
@njbennett
Copy link
Copy Markdown
Contributor Author

Github not being aware that I've signed the CLA doesn't seem to stop me from merging things so 😬

@tjvman tjvman deleted the honeeycomb-pr branch August 24, 2021 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants