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

feat: Add ActiveStorage instrumentation #1313

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

ymtdzzz
Copy link
Contributor

@ymtdzzz ymtdzzz commented Dec 20, 2024

Description

Adding support for instrumenting ActiveStorage using ActiveSupport::Notification.

related: #1307

Test

Example script

$ cd instrumentation/active_storage/example
$ ruby trace_demonstration.rb

#<struct OpenTelemetry::SDK::Trace::SpanData
 name="service_upload.active_storage",
 kind=:internal,
 status=#<OpenTelemetry::Trace::Status:0x000000012b2df308 @code=1, @description="">,
 parent_span_id="\x00\x00\x00\x00\x00\x00\x00\x00",
 total_recorded_attributes=2,
 total_recorded_events=0,
 total_recorded_links=0,
 start_timestamp=1736671214398000000,
 end_timestamp=1736671214398395000,
 attributes={"active_storage.checksum"=>"x4UGDIZnlswqFwjJlxVMjg==", "active_storage.service"=>"Disk"},
 links=nil,
 events=nil,
 resource=
  #<OpenTelemetry::SDK::Resources::Resource:0x000000011f5bd810
   @attributes=
    {"service.name"=>"unknown_service",
     "process.pid"=>98542,
     "process.command"=>"trace_demonstration.rb",
     "process.runtime.name"=>"ruby",
     "process.runtime.version"=>"3.2.5",
     "process.runtime.description"=>"ruby 3.2.5 (2024-07-26 revision 31d0f1a2e7) [arm64-darwin23]",
     "telemetry.sdk.name"=>"opentelemetry",
     "telemetry.sdk.language"=>"ruby",
     "telemetry.sdk.version"=>"1.6.0"}>,
 instrumentation_scope=#<struct OpenTelemetry::SDK::InstrumentationScope name="OpenTelemetry::Instrumentation::ActiveStorage", version="0.0.0">,
 span_id="\xCA$\xA1\xD6\x01RV\x95",
 trace_id="\x13~\xDF\x02\x1F\x8C\xF3\x8B\x96\xA0\x97\xC3)}\x9A\xFA",
 trace_flags=#<OpenTelemetry::Trace::TraceFlags:0x000000012a539050 @flags=1>,
 tracestate=#<OpenTelemetry::Trace::Tracestate:0x000000012a536508 @hash={}>>

With other instrumentations in your Rails app

Without ActiveStorage instrumentation

# config/initializers/opentelemetry.rb
require "opentelemetry/sdk"

# install all compatible instrumentation with default configuration
OpenTelemetry::SDK.configure do |c|
  c.service_name = "otel-ruby-development"
  c.use "OpenTelemetry::Instrumentation::ActionPack"
  c.use "OpenTelemetry::Instrumentation::ActiveRecord"
  # c.use "OpenTelemetry::Instrumentation::ActiveStorage"
  c.use "OpenTelemetry::Instrumentation::ActiveJob"
end

image

With ActiveStorage instrumentation

# config/initializers/opentelemetry.rb
require "opentelemetry/sdk"

# install all compatible instrumentation with default configuration
OpenTelemetry::SDK.configure do |c|
  c.service_name = "otel-ruby-development"
  c.use "OpenTelemetry::Instrumentation::ActionPack"
  c.use "OpenTelemetry::Instrumentation::ActiveRecord"
  c.use "OpenTelemetry::Instrumentation::ActiveStorage", { key: :include, url: :include }
  c.use "OpenTelemetry::Instrumentation::ActiveJob"
end

image
image

@ymtdzzz ymtdzzz force-pushed the feature/active_storage_support branch 6 times, most recently from 7a91acc to 641f823 Compare January 13, 2025 01:58
@ymtdzzz ymtdzzz changed the title [WIP] feat: Add ActiveStorage instrumentation feat: Add ActiveStorage instrumentation Jan 14, 2025
@ymtdzzz ymtdzzz force-pushed the feature/active_storage_support branch 2 times, most recently from 721c056 to 5959af8 Compare January 14, 2025 12:43
@ymtdzzz ymtdzzz force-pushed the feature/active_storage_support branch from 5959af8 to cf7ee82 Compare January 14, 2025 14:23
@ymtdzzz ymtdzzz marked this pull request as ready for review January 14, 2025 14:45
Copy link
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

Thank you, @ymtdzzz! I'm looking forward to OTel to offering ActiveStorage support.

Copy link
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

Thanks for all the updates! 😄

The last thing I can think of would be to update the CODEOWNERS file to add your name to the list of reviewers. The OpenTelemetry Ruby team needs help from end users and other experts in the community to maintain our instrumentation. Since you built this Active Storage instrumentation, we'd love for you to help us maintain the library going forward. By becoming a CODEOWNER, you would be added as a reviewer to any PR opened to update the instrumentaiton/active_storage code. We may also tag you directly in issues raised regarding the Active Storage instrumentation.

Would you be comfortable with this?

I'd also like @xuan-cao-swi to take another look when he has a chance.

@ymtdzzz
Copy link
Contributor Author

ymtdzzz commented Jan 22, 2025

@kaylareopelle

Thank you for the suggestion!

Of course I'm happy to help with reviews and issues regarding the instrumentation!
I've added my name to the CODEOWNERS in b5a65bd .

I'm looking forward to collaborating with the team!

@ymtdzzz ymtdzzz force-pushed the feature/active_storage_support branch from 9221f0d to b5a65bd Compare January 22, 2025 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants