apm-agent-rum-js icon indicating copy to clipboard operation
apm-agent-rum-js copied to clipboard

feat: add pageLoadParentId configuration

Open devcorpio opened this issue 3 years ago • 5 comments

Context

One of the requirements for Crosslinking Synthetics with APM issue was to allow the RUM agent the creation of the page-load transaction as child of the one created in Synthetics.

The main goal of that is to provide visibility into how the synthetic journeys are executed and what actions are happening inside every step. This image extracted from the issue linked above it's a great example.

Summary

We expose a new agent configuration option named pageLoadParentId whose value will be the id of the transaction that we want to establish as parent of the page-load transaction. Before this PR page-load transaction was always treated as the root transaction, now it will be possible to set a parent if needed thanks to this new config.

devcorpio avatar Mar 16 '22 18:03 devcorpio

:green_heart: Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-03-23T14:03:17.246+0000

  • Duration: 70 min 28 sec

Test stats :test_tube:

Test Results
Failed 0
Passed 774
Skipped 8
Total 782

:robot: GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run benchmark tests : Run the benchmark test.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

apmmachine avatar Mar 16 '22 18:03 apmmachine

/test

devcorpio avatar Mar 22 '22 10:03 devcorpio

:package: Bundlesize report

Filename Size(bundled) Size(gzip) Diff(gzip)
elastic-apm-opentracing.umd.min.js 66.1 KiB 21.0 KiB :warning: 29 Bytes
elastic-apm-rum.umd.min.js 60.0 KiB 19.4 KiB :warning: 29 Bytes

apmmachine avatar Mar 22 '22 10:03 apmmachine

Thanks @devcorpio Changes LGTM, But we need to do few more things

  1. Add the limit check to truncation logic that should truncate if the field is past 1024 bytes- https://github.com/elastic/apm-agent-rum-js/blob/434d6213a46c035680821b7843a15f628217ec64/packages/rum-core/src/common/truncate.js#L95
  2. Document the pageloadParentId in the config option along with other trace options- https://github.com/elastic/apm-agent-rum-js/blob/main/docs/configuration.asciidoc#pageloadtraceid

I would also like to test with the Synthetics APM PR before we close this down. Thanks!

The truncation logic it's already working. The function truncate default limit is 1024 as we can see here.

Something that caught my attention is that if I replace parent_id: true with parent_id: [KEYWORD_LIMIT, true] and the value for parent_id is empty the truncate logic will set the N/A value which in my opinion feels a bit weird value for a parent_id.

One way to avoid that is setting parent_id: [KEYWORD_LIMIT, false] but seeing that the value was already being truncated, I would advocate for letting the config as it is. What do you think?

devcorpio avatar Mar 23 '22 11:03 devcorpio

One way to avoid that is setting parent_id: [KEYWORD_LIMIT, false] but seeing that the value was already being truncated, I would advocate for letting the config as it is. What do you think?

No, we dont need to set N/A and that flag exists only when we cannot send a empty string to the APM server which are marked as required: true in the server spec.

vigneshshanmugam avatar Mar 23 '22 15:03 vigneshshanmugam