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

Add instrumentation patch metrics #871

Merged
merged 5 commits into from
Dec 5, 2019

Conversation

delner
Copy link
Contributor

@delner delner commented Nov 28, 2019

As a further extension of #867, this pull request refactors how integration patching works, and adds a metric for whenever integration patching succeeds/fails.

New metrics include:

  • datadog.tracer.instrumentation_patched: Whenever a Datadog::Contrib::Patcher#patch succeeds.
    • patcher: Patcher that was applied.
    • target_version: Version of software that was patched (typically a gem version.)
  • datadog.tracer.error.instrumentation_patch: Whenever a Datadog::Contrib::Patcher#patch raises an error.
    • patcher: Patcher that was attempted.
    • target_version: Version of software that patch was attempted against (typically a gem version.)
    • error: Class name of error raised.

Screenshot from 2019-12-02 18-26-22

@delner delner added integrations Involves tracing integrations do-not-merge/WIP Not ready for merge feature Involves a product feature labels Nov 28, 2019
@delner delner requested a review from marcotc November 28, 2019 06:19
@delner delner self-assigned this Nov 28, 2019
@delner delner force-pushed the feature/instrumentation_patch_metrics branch from 7caf121 to a2dc440 Compare December 2, 2019 18:38
@delner delner force-pushed the feature/additional_health_metrics branch from 89386fd to 0f99b5e Compare December 2, 2019 18:43
@delner delner force-pushed the feature/instrumentation_patch_metrics branch from a2dc440 to 4873288 Compare December 2, 2019 18:49
@delner delner changed the base branch from feature/additional_health_metrics to master December 2, 2019 19:35
@delner delner force-pushed the feature/instrumentation_patch_metrics branch from 4873288 to 7355d4c Compare December 2, 2019 21:12
@delner delner marked this pull request as ready for review December 2, 2019 21:13
@delner delner requested a review from a team December 2, 2019 21:13
@delner delner removed the do-not-merge/WIP Not ready for merge label Dec 2, 2019
@delner
Copy link
Contributor Author

delner commented Dec 2, 2019

This is now ready for review.

I think this is fine as is, but it might be worthwhile also adding a "gem version" tag to the metric of the library that failed to patch. e.g. If our Rails integration failed to patch Rails 6.1, it would add a target_version:6.1.0 tag to the error metric.

base.send(:include, InstanceMethods)

base.singleton_class.send(:prepend, CommonMethods)
base.send(:prepend, CommonMethods) if base.class == Class
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason for handling the case where the Patcher is a class, given all our current Patchers are modules?
Would it be for future proofing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, technically our integration API doesn't require all patchers to be modules, just that they include Datadog::Contrib::Patcher and behave like one. This means users who have created custom integrations might have implemented the patchers as classes: it's more of a "avoiding possible breaking change" thing than future proofing.

marcotc
marcotc previously approved these changes Dec 2, 2019
@delner delner force-pushed the feature/instrumentation_patch_metrics branch from de3616c to 280b180 Compare December 2, 2019 22:39
@delner
Copy link
Contributor Author

delner commented Dec 2, 2019

Added target_version tag and an instrumentation_patched metric which is emitted whenever an integration's patch succeeds.

marcotc
marcotc previously approved these changes Dec 3, 2019
@delner delner force-pushed the feature/instrumentation_patch_metrics branch from f3e84d8 to c11f784 Compare December 3, 2019 19:10
@marcotc marcotc merged commit b1c1c42 into master Dec 5, 2019
@marcotc marcotc deleted the feature/instrumentation_patch_metrics branch December 5, 2019 18:17
@marcotc marcotc added this to the 0.30.0 milestone Dec 5, 2019
@marcotc marcotc mentioned this pull request Jan 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Involves a product feature integrations Involves tracing integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants