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: Record original user agent for spans and logs #1358

Merged
merged 7 commits into from
Oct 3, 2024

Conversation

MikeGoldsmith
Copy link
Contributor

Which problem is this PR solving?

Refinery loses the original user agent of the sending application because it aggregates spans until a trace decision is made. This PR records the original user agent for both HTTP and gRPC requests to the event, batch and OTLP trace/log endpoints in a new metadata field meta.refinery.incoming_user_agent.

Short description of the changes

  • Update event, batch, OTLP trace/logs endpoints to record original user agent in new field
  • Update tests to verify events have the new field set
  • Add defer for bad API key test to restore allowed keys so subsequent tests can pass

@MikeGoldsmith MikeGoldsmith added the type: enhancement New feature or request label Oct 2, 2024
@MikeGoldsmith MikeGoldsmith self-assigned this Oct 2, 2024
@MikeGoldsmith MikeGoldsmith requested a review from a team as a code owner October 2, 2024 13:04
@MikeGoldsmith MikeGoldsmith changed the title Mike/incoming user agent feat: Record original user agent for spans and logs Oct 2, 2024
@MikeGoldsmith MikeGoldsmith added this to the v2.9 milestone Oct 2, 2024
@cartermp
Copy link
Member

cartermp commented Oct 2, 2024

We'll also want to make an update in shepherd telemetry to capture this like we do the user_agents strings today.

Copy link
Contributor

@VinozzZ VinozzZ left a comment

Choose a reason for hiding this comment

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

LGTM

route/otlp_trace_test.go Show resolved Hide resolved
route/route.go Outdated Show resolved Hide resolved
@MikeGoldsmith
Copy link
Contributor Author

Looks like this has broken with the new trace locality feature. I'll work on fixing tests.

@MikeGoldsmith MikeGoldsmith merged commit ea00691 into main Oct 3, 2024
5 checks passed
@MikeGoldsmith MikeGoldsmith deleted the mike/incoming-user-agent branch October 3, 2024 13:38
MikeGoldsmith added a commit that referenced this pull request Oct 4, 2024
## Which problem is this PR solving?

Refinery loses the original user agent of the sending application
because it aggregates spans until a trace decision is made. This PR
records the original user agent for both HTTP and gRPC requests to the
event, batch and OTLP trace/log endpoints in a new metadata field
`meta.refinery.incoming_user_agent`.

- Closes #1356

## Short description of the changes
- Update event, batch, OTLP trace/logs endpoints to record original user
agent in new field
- Update tests to verify events have the new field set
- Add defer for bad API key test to restore allowed keys so subsequent
tests can pass
# Conflicts:
#	app/app_test.go
#	route/route.go
MikeGoldsmith added a commit that referenced this pull request Oct 7, 2024
## Which problem is this PR solving?

Following up to the following PR, we should only set the incoming user
agent if the key does not already exist. This is useful in certain
scenarios when two Refinery's are connected together.

- #1358

## Short description of the changes
- Only set incoming user agent meta field if the event doesn't already
have a value for it
- Add unit test to verify behaviour
- Update existing tests to set custom user-agent and verify it's set
correctly
MikeGoldsmith added a commit that referenced this pull request Oct 7, 2024
## Which problem is this PR solving?

Following up to the following PR, we should only set the incoming user
agent if the key does not already exist. This is useful in certain
scenarios when two Refinery's are connected together.

- #1358

## Short description of the changes
- Only set incoming user agent meta field if the event doesn't already
have a value for it
- Add unit test to verify behaviour
- Update existing tests to set custom user-agent and verify it's set
correctly
# Conflicts:
#	app/app_test.go
#	route/route_test.go
TylerHelmuth pushed a commit that referenced this pull request Oct 16, 2024
Refinery loses the original user agent of the sending application
because it aggregates spans until a trace decision is made. This PR
records the original user agent for both HTTP and gRPC requests to the
event, batch and OTLP trace/log endpoints in a new metadata field
`meta.refinery.incoming_user_agent`.

- Closes #1356

- Update event, batch, OTLP trace/logs endpoints to record original user
agent in new field
- Update tests to verify events have the new field set
- Add defer for bad API key test to restore allowed keys so subsequent
tests can pass
TylerHelmuth pushed a commit that referenced this pull request Oct 16, 2024
Following up to the following PR, we should only set the incoming user
agent if the key does not already exist. This is useful in certain
scenarios when two Refinery's are connected together.

- #1358

- Only set incoming user agent meta field if the event doesn't already
have a value for it
- Add unit test to verify behaviour
- Update existing tests to set custom user-agent and verify it's set
correctly
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.

Record original user agent in a meta field
3 participants