honeycombio / refinery

Refinery is a trace-aware tail-based sampling proxy. It examines whole traces and intelligently applies sampling decisions (whether to keep or discard) to each trace.
285 stars 88 forks source link

Allow setting API Key on Refinery #634

Closed McSick closed 4 weeks ago

McSick commented 1 year ago

Is your feature request related to a problem? Please describe. Right now, refinery requires api key be attached to the incoming events. There could be a scenario where many OTel collectors are in place behind refinery. Each having to maintain an API Key to Honeycomb.

Describe the solution you'd like Instead, refinery is the endpoint anyways, if it could have a setting to use an API Key in the refinery config and use that when sending events to Honeycomb, there is now just 1 place to have to maintain an API Key.

Describe alternatives you've considered Putting a proxy after refinery, hijacking the requests, and attaching the api key there.

Additional context

kentquirk commented 1 year ago

I like this idea.

One problem we'll have to document -- if you give Refinery a key that doesn't have permission to create datasets, the only feedback we can give you if you hook up a new service is to put it into the logs (since you can't push errors all the way back to the sending application). At the very least, we should check key permissions on startup and log a warning if you don't have create.

Another issue is whether we have anyone who will want to specify different keys based on service.name. In other words, they want a key per service but want to manage it centrally. I'm going to assume no for now, but please correct me if I'm wrong.

Design

We'll have a new configuration value -- maybe HoneycombAPIKey? -- that, if specified, is the key to use for requests that do not contain a key of their own.

If they do contain a key of their own, I think we should use the existing APIKeys config value and add some new semantics. Today, APIKeys contains explicit keys or just *. If * is present, then all keys are allowed.

The new semantics if the HoneycombAPIKey is present would be:

This is basically what we have today, but with the added bonus of key replacement if that's what you want.

kentquirk commented 1 year ago

I worked on this some, and talked with design about it, and there are some additional problems (basically, what do we do for key replacement in a multi-environment world?). I still think it's possible but I want to save it for after v2.0 to properly consider later.

cartermp commented 11 months ago

Discussion had - we think this would be good to have as mentioned above!

kentquirk commented 1 month ago

Proposal: New config field in AccessKeys: SendKey must be a valid Honeycomb ingest key. UseSendKeyForAll is a boolean value; if true, telemetry sent to Honeycomb will be sent with this key instead of any key that was attached. (We need the separate bool to help diagnose key problems) UseSendKeyIfMissing will not replace existing keys but will inject it if the incoming key is missing.

(still needs further thought)