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

aws-sdk instrumentation adapter #13

Closed
mwear opened this issue May 12, 2020 · 7 comments
Closed

aws-sdk instrumentation adapter #13

mwear opened this issue May 12, 2020 · 7 comments
Labels
help wanted Extra attention is needed instrumentation

Comments

@mwear
Copy link
Member

mwear commented May 12, 2020

Convert Datadog instrumentation for aws-sdk to OpenTelemetry.

See

@mwear mwear added feature New feature or request help wanted Extra attention is needed labels May 12, 2020
@fbogsany fbogsany added instrumentation and removed feature New feature or request labels Feb 22, 2021
@YanivD
Copy link
Member

YanivD commented Oct 28, 2021

Hi 👋

I'm working to create an OpenTelemetry ruby instrumentation for aws-sdk.
It's going to be my first ruby instrumentation and I'm looking for guidance (I created Node Elasticsearch instrumentation few month ago)

I wonder if there's any resources for creating instrumentation in Ruby? I'm looking for guidelines for setup the dev environment, best practices and such topics.

If someone can be available and responsive it can be really helpful. Maybe I should ask the questions in the otel-ruby slack channel?

Thanks

@arielvalentin
Copy link
Collaborator

We don't have explicit guidelines at the moment.

Some things that have come up in the past:

  • use instrumentation-base. It provides lifecycle hooks to ensure the instrumentation can be loaded
  • target a specific minimum gem version to ensure your patches work
  • We're still working through this but I think we prefer building 1:1 instrumentations for complex libraries (like aws-sdk) instead of instrumenting everything in one gem. See Rails auto-instrumentation for an example.
  • prefer using hooks or features of the target library over monkey patching
  • use Module.prepend and avoid using aliasing methods on target classes
  • allocate the minimal amount of objects and methods you need to satisfy the instrumentation. We want to avoid overhead and minimize GC
  • instrumentation may raise errors when installed but must not raise errors when in use
  • use defined semantic conventions whenever possible. Use namespaces for attributes that are specific to the target gem. Look around at other language implementations for similar instruments ions to see if there are key names you could reuse and submit upstream to the spec
  • We are still fleshing out what to do in circumstances where there is duplication between auto instrumented libraries. There may be some churn and PR feedback here on using Common attributes. Thanks in advance for your patience.
  • write automated tests that verify your changes to provide a safety net and avoid regressions
  • define an appraisal specification that tests against all supported versions

@YanivD
Copy link
Member

YanivD commented Nov 1, 2021

@arielvalentin Thanks for your comment.
Is there any automated process to test the instrumentation against all aws-sdk-ruby versions?

@YanivD
Copy link
Member

YanivD commented Nov 1, 2021

In JS aws-sdk instrumentation there's an option to suppressInternalInstrumentation. When setting true, it hides all underlying http spans.

Do you think it should be also an option in aws-sdk-ruby instrumentation? I couldn't find any ruby instrumentation with such option.

@arielvalentin
Copy link
Collaborator

@arielvalentin Thanks for your comment. Is there any automated process to test the instrumentation against all aws-sdk-ruby versions?

@YanivD You do not have to test every version, only those you intend on supporting.

This repository uses the Appraisals gem combined with GitHub Actions to test multiple versions of instrumentations. The CI builds will automatically detect your appraisal file and test it against different versions on your behalf. Here is an example in the Rack Instrumentation:

https://github.com/open-telemetry/opentelemetry-ruby/blob/main/instrumentation/rack/Appraisals

@arielvalentin
Copy link
Collaborator

In JS aws-sdk instrumentation there's an option to suppressInternalInstrumentation. When setting true, it hides all underlying http spans.

Do you think it should be also an option in aws-sdk-ruby instrumentation? I couldn't find any ruby instrumentation with such option.

I think that option would be useful.

There is a utility method untraced that you may use but it will only suppress the immediate child span. Depending on the complexity of the auto-instrumented dependencies and sampling settings you may still some child spans appear.

https://github.com/open-telemetry/opentelemetry-ruby/blob/467dc091520dc3f308d40ccb0a216810a5ff051b/common/lib/opentelemetry/common/utilities.rb#L75

@YanivD
Copy link
Member

YanivD commented Nov 21, 2021

I believe this issue can be closed now, after open-telemetry/opentelemetry-ruby#1014 merged

@plantfansam plantfansam transferred this issue from open-telemetry/opentelemetry-ruby Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed instrumentation
Projects
None yet
Development

No branches or pull requests

4 participants