opentelemetry-ruby-contrib icon indicating copy to clipboard operation
opentelemetry-ruby-contrib copied to clipboard

aws-sdk instrumentation adapter

Open mwear opened this issue 4 years ago • 7 comments

Convert Datadog instrumentation for aws-sdk to OpenTelemetry.

See

mwear avatar May 12 '20 00:05 mwear

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

YanivD avatar Oct 28 '21 12:10 YanivD

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

arielvalentin avatar Oct 28 '21 13:10 arielvalentin

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

YanivD avatar Nov 01 '21 05:11 YanivD

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.

YanivD avatar Nov 01 '21 14:11 YanivD

@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 avatar Nov 01 '21 15:11 arielvalentin

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

arielvalentin avatar Nov 01 '21 15:11 arielvalentin

I believe this issue can be closed now, after https://github.com/open-telemetry/opentelemetry-ruby/pull/1014 merged

YanivD avatar Nov 21 '21 08:11 YanivD