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

Add Environment & Services support #403

Merged
merged 39 commits into from
Feb 17, 2022
Merged

Add Environment & Services support #403

merged 39 commits into from
Feb 17, 2022

Conversation

MikeGoldsmith
Copy link
Contributor

@MikeGoldsmith MikeGoldsmith commented Feb 15, 2022

Which problem is this PR solving?

Adds Environments & Services support to Refinery. This enables users to define environments in their rules configurations, much like a dataset, with custom sampler, sample rate, etc.

Environment names are defined in the rules configuration using the slugified version of the environment name, eg the Environment name "My Cool Company-Production" would become "my_cool_company_production".

Environment names are looked up in the Honeycomb API using the request's API key and then cached for subsequent requests. Environment names are cached for 1 hour.

TODO:

  • Add support for configuring environment cache length
  • Add tests for existing endpoints with new API keys
  • Add tests for EnvironmentCache
  • revert the pinned version

Future:

  • Review & add default environment to allow env. and dataset with same name in rules file

  • Add slugify logic so non-slugified names can be used in rules config, or update Honeycomb API to return un-slugified environment name

  • Closes Add support for environment & services #401

Short description of the changes

  • Updates Husky to latest (v0.9.0) and update OTLP trace handler to work with batches per service-name
    • This is done once per endpoint, not per event, to prevent unnecessary cache hits
  • Update Trace struct to have environment name and GetSamplerKey which returns either the dataset for legacy keys, or environment for non-legacy keys
  • When receiving events via OTLP-trace, HTTP /event or /batch, we check the API key and if not legacy (eg 32 chars), lookup environment and add to trace
  • Environment lookup is done using the /1/auth Honeycomb API endpoint and result is cached
  • Added EnvironmentCache which uses a RWMutex to read/write safety to the underlying map[string]cacheItem where cacheItem holds a TTL and environment name
  • Added EnvironmentCacheTTL config option with default of 1 hour "1h"
  • Use Trace.GetSamplerKey to resolve rules implementation
  • Updates existing tests that expect legacy key to provide a key with 32 characters

@MikeGoldsmith MikeGoldsmith self-assigned this Feb 15, 2022
@MikeGoldsmith MikeGoldsmith added type: enhancement New feature or request version: bump minor A PR that adds behavior, but is backwards-compatible. labels Feb 15, 2022
@vreynolds
Copy link
Contributor

looks good, just some optimization suggestions. I checked it out locally and did a cursory test with legacy key + dataset-based rules, and new key + environment-based rules

Copy link
Contributor

@kentquirk kentquirk left a comment

Choose a reason for hiding this comment

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

My review was mostly limited to looking over the cache stuff.

route/route.go Show resolved Hide resolved
route/route.go Show resolved Hide resolved
@MikeGoldsmith MikeGoldsmith marked this pull request as ready for review February 17, 2022 11:53
@MikeGoldsmith MikeGoldsmith requested a review from a team February 17, 2022 11:54
@MikeGoldsmith MikeGoldsmith changed the title E&S part 2 Add Environment & Services support Feb 17, 2022
Copy link
Contributor Author

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

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

Need to do a couple of tidy up things before merging

build-pkg.sh Outdated Show resolved Hide resolved
app/app_test.go Outdated Show resolved Hide resolved
@MikeGoldsmith MikeGoldsmith merged commit e5aa8cd into main Feb 17, 2022
@MikeGoldsmith MikeGoldsmith deleted the mike/e&s_simple branch February 17, 2022 20:05
ghost pushed a commit to opsramp/tracing-proxy that referenced this pull request Jul 5, 2024
Co-authored-by: Vera Reynolds <verareynolds@honeycomb.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request version: bump minor A PR that adds behavior, but is backwards-compatible.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants