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: Sanitise headers to OTel semconv and add as separate fields/attributes #296

Merged
merged 3 commits into from
Oct 23, 2023

Conversation

MikeGoldsmith
Copy link
Contributor

@MikeGoldsmith MikeGoldsmith commented Oct 23, 2023

Which problem is this PR solving?

The OTel semantic conventions for headers is to lowercase and replace - with _ and for each header to be a separate field/attribute. The logic to do the sanitisation was added in #290 for just the OTel handler.

This PR moves it to a utility func that both the OTel and libhoney handlers can use.

Short description of the changes

  • Move sanitisation logic to shared utility func
  • Use new func in both otel and libhoney handlers
  • Update both handler tests
  • Add -count=1 to test command to clear cache on test runs

How to verify that this has the expected result

Both libhoney and OTel handlers sanitise headers and add each header as a separate field / attribute.

@MikeGoldsmith MikeGoldsmith added the type: enhancement New feature or request label Oct 23, 2023
@MikeGoldsmith MikeGoldsmith self-assigned this Oct 23, 2023
@MikeGoldsmith MikeGoldsmith requested a review from a team October 23, 2023 16:40
@JamieDanielson
Copy link
Contributor

I added -count=1 to test command to clear cache on test runs, and updated the PR description as it failed on my minor change and it shouldn't have. This is the idiomatic way to clear cache on test runs.

Copy link
Contributor

@JamieDanielson JamieDanielson left a comment

Choose a reason for hiding this comment

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

Looks great!
Here's an example with HTTP_HEADERS set to "User-Agent,Accept,Content-Type"

shared-header-logic

@JamieDanielson JamieDanielson merged commit d8d1fa5 into main Oct 23, 2023
4 checks passed
@JamieDanielson JamieDanielson deleted the mike/headers branch October 23, 2023 19:23
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants