Add view_component integration for Datadog::Tracing
#4977
Conversation
* `ViewComponent` supports instrumentation through `ActiveSupport` notifications, and having the ability to trace a ViewComponent using Datadog's APM provides a similar level of debugging ease that tracing `ActionView` template and partial rendering provides * This tracing component was based on `Datadog::Tracing::Contrib::ActionView`, with some key differences: * There is only one action available for instrumentation: `render` * ViewComponent is technically its own component, not part of Rails itself * Therefore, it uses `Contrib::Integration#auto_instrument?` rather than checking the Railtie * The `span.resource` is the name of the component (`MyComponent`), not its identifier (`my_component.rb`), for a better user experience * Added tests, but need to update the Matrixfile and appraisals
* I was not able to get the `docker compose` setup running on my machine due to the inability to pull the `ddapm-test-agent` * Therefore, I was not able to spool up containers for each version of Ruby to generate their `gemfiles/` * While I tested this locally with `ruby-3.3`, I did not commit the changes since I was not able to follow the containerized setup
There was a problem hiding this comment.
@tcannonfodder thank you for your contribution!
I left some small remarks, and I think it would make sense to add an integration test for this feature. If you need some help with the integration test, please let me know.
| end | ||
|
|
||
| option :service_name | ||
| option :component_base_path do |o| |
There was a problem hiding this comment.
Would components_base_path be better? Since it's a path containing multiple components.
There was a problem hiding this comment.
I also think we would need to ensure the correct format here - when calculating the component identifier, we are relying that this path will end with /, otherwise component identifiers will start with a /
There was a problem hiding this comment.
Both this (and the comment below) are essentially straight ports from the action_view integration; which felt like the right approach to take here because these components are run together (you often render ViewComponents inside ActionView), so it feels right that they are configured and behave similarly.
But! Y’all have the final say here; so just lemme know what makes sense for the repo. :) I just wanted to explain my reasoning for these decisions.
| sections_view = identifier.split(base_path) | ||
|
|
||
| if sections_view.length == 1 | ||
| identifier.split('/')[-1] |
There was a problem hiding this comment.
I think this won't work if the components base path is repeated somewhere in the filename:
> 'some/path/components/admin/components/sidebar_component.rb'.split('components/')[-1]
=> "sidebar_component.rb"
| span.set_tag(Ext::TAG_COMPONENT_NAME, payload[:name]) | ||
|
|
||
| if (identifier = Utils.normalize_component_identifier(payload[:identifier])) | ||
| span.set_tag(Ext::TAG_COMPONENT_IDENTIFIER, identifier) |
There was a problem hiding this comment.
I'm not sure this really is an identifier, since it's basically a partial file path. Do we want to name it something like component_path?
There was a problem hiding this comment.
We definitely could; I had based the name component on the value that's passed in the ActiveSupport's payload; so that the APM telemetry matched the instrumentation data. But if you'd like me to change it; just let me know.
Reference: https://viewcomponent.org/guide/instrumentation.html
Thanks so much for reviewing it! Eager to get this merged in 😄 . I left some additional contextual comments for why I’d made those decisions; just let me know which approaches you’d like me to take. I’ll try writing an integration test; and I’ll make a new comment with a WIP commit if I get stuck. |
2 participants
What does this PR do?
Add
view_componentintegration forDatadog::TracingViewComponentsupports instrumentation throughActiveSupportnotifications, and having the ability to trace a ViewComponent using Datadog's APM provides a similar level of debugging ease that tracingActionViewtemplate and partial rendering providesDatadog::Tracing::Contrib::ActionView, with some key differences:renderContrib::Integration#auto_instrument?rather than checking the Railtiespan.resourceis the name of the component (MyComponent), not its identifier (my_component.rb), for a better user experienceWIP: Add
view_componentbuild matricies in appraisaldocker composesetup running on my machine due to the inability to pull theddapm-test-agentgemfiles/ruby-3.3, I did not commit the changes since I was not able to follow the containerized setupMotivation:
I'm using
ViewComponentin my apps, and wanted to be able to debug performance issues inside of a ViewComponent.Change log entry
Additional Notes:
Example screenshot:
How to test the change?
Tests have been added based on the
action_viewintegration