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

chore: add contextual attributes to faro measurement #4975

Merged
merged 4 commits into from
Aug 31, 2023

Conversation

codecapitano
Copy link
Contributor

PR Description

In the Faro Web-SDK we added an option to provide additional contextual attributes to the measurements payload.
To enable this we need to update the agent as well.

Which issue(s) this PR fixes

Notes to the Reviewer

Just for reference here are the PRs which make the needed changes to the Faro Receiver and the Faro Web-SDK.

PR Checklist

  • CHANGELOG.md updated
  • Tests updated

@codecapitano codecapitano requested a review from a team as a code owner August 28, 2023 16:03
@codecapitano codecapitano added app-agent-receiver Related to the app-agent-receiver go Pull requests that update Go code labels Aug 28, 2023
Copy link
Member

@rlankfo rlankfo left a comment

Choose a reason for hiding this comment

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

LGTM for the MeasurementContext. I had one question about adding Type but I'm not sure if it's necessary in this PR.

pkg/integrations/v2/app_agent_receiver/payload.go Outdated Show resolved Hide resolved
@rlankfo
Copy link
Member

rlankfo commented Aug 28, 2023

@codecapitano looks like there are some additional test cases we should update:

--- FAIL: TestExportLogs (0.01s)
    logs_exporter_test.go:103: 
        	Error Trace:	/Users/runner/work/agent/agent/pkg/integrations/v2/app_agent_receiver/logs_exporter_test.go:103
        	Error:      	Not equal: 
        	            	expected: "timestamp=\"2021-09-30 10:46:17.68 +0000 UTC\" kind=measurement ttfb=14.000000 ttfcp=22.120000 ttfp=20.120000 traceID=abcd spanID=def sdk_name=grafana-frontend-agent sdk_version=1.0.0 app_name=testapp app_release=0.8.2 app_version=abcdefg app_environment=production user_email=geralt@kaermorhen.org user_id=123 user_username=domasx2 user_attr_foo=bar session_id=abcd session_attr_time_elapsed=100s page_url=https://example.com/page browser_name=chrome browser_version=88.12.1 browser_os=linux browser_mobile=false view_name=foobar"
        	            	actual  : "timestamp=\"2021-09-30 10:46:17.68 +0000 UTC\" kind=measurement ttfb=14.000000 ttfcp=22.120000 ttfp=20.120000 traceID=abcd spanID=def context_hello=world sdk_name=grafana-frontend-agent sdk_version=1.0.0 app_name=testapp app_release=0.8.2 app_version=abcdefg app_environment=production user_email=geralt@kaermorhen.org user_id=123 user_username=domasx2 user_attr_foo=bar session_id=abcd session_attr_time_elapsed=100s page_url=https://example.com/page browser_name=chrome browser_version=88.12.1 browser_os=linux browser_mobile=false view_name=foobar"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-timestamp="2021-09-30 10:46:17.68 +0000 UTC" kind=measurement ttfb=14.000000 ttfcp=22.120000 ttfp=20.120000 traceID=abcd spanID=def sdk_name=grafana-frontend-agent sdk_version=1.0.0 app_name=testapp app_release=0.8.2 app_version=abcdefg app_environment=production user_email=geralt@kaermorhen.org user_id=123 user_username=domasx2 user_attr_foo=bar session_id=abcd session_attr_time_elapsed=100s page_url=https://example.com/page browser_name=chrome browser_version=88.12.1 browser_os=linux browser_mobile=false view_name=foobar
        	            	+timestamp="2021-09-30 10:46:17.68 +0000 UTC" kind=measurement ttfb=14.000000 ttfcp=22.120000 ttfp=20.120000 traceID=abcd spanID=def context_hello=world sdk_name=grafana-frontend-agent sdk_version=1.0.0 app_name=testapp app_release=0.8.2 app_version=abcdefg app_environment=production user_email=geralt@kaermorhen.org user_id=123 user_username=domasx2 user_attr_foo=bar session_id=abcd session_attr_time_elapsed=100s page_url=https://example.com/page browser_name=chrome browser_version=88.12.1 browser_os=linux browser_mobile=false view_name=foobar
        	Test:       	TestExportLogs

Copy link
Member

@rlankfo rlankfo left a comment

Choose a reason for hiding this comment

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

test cases + changelog formatting

CHANGELOG.md Outdated Show resolved Hide resolved
@codecapitano codecapitano requested a review from rlankfo August 29, 2023 16:01
Copy link
Member

@rlankfo rlankfo left a comment

Choose a reason for hiding this comment

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

lgtm!

@codecapitano codecapitano force-pushed the add-contextual-attributes-to-faro-measurements branch from 58f2b8d to 2a714f0 Compare August 30, 2023 07:45
@marctc marctc merged commit acd6181 into main Aug 31, 2023
@marctc marctc deleted the add-contextual-attributes-to-faro-measurements branch August 31, 2023 08:54
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 22, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
app-agent-receiver Related to the app-agent-receiver frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants