Skylight::Extensions::SourceLocation causes sustained p90 spike

[colbymelvin]

Hello, I wanted to report an issue that my org ran into when upgrading to v5.0.0.

We read through the changelogs and noted the new Skylight::Extensions::SourceLocation being enabled by default. We assumed this was fine, and did not set SKYLIGHT_ENABLE_SOURCE_LOCATIONS=false.

Immediately after deploy, we saw p90 request durations spike to roughly 10x, followed by a sustained average of 2x normal duration after settling. This appears to mostly be caused by a considerable increase in Ruby execution time on every request. Post rollback, we saw our p90 durations consolidate around their normal patterns.

I've provided a screenshot. Note that the first vertical dotted line represents the deploy contains the Skylight v5.0.0 update. The last vertical dotted line represents the deploy containing the rollback.

My understanding from reading the code is that Skylight::Extensions::SourceLocation is an experimental feature - should it be disabled by default?

At the very least, should the experimental nature of Skylight::Extensions::SourceLocation be highlighted somewhere? One has to go digging through the docs to find mention of this new feature, but the docs don't mention that it's experimental. The only indication I can find that alludes to Skylight::Extensions::SourceLocation being experimental is a single line of code in a spec.

[booleanbetrayal]

FWIW - Rolling 5.0.x with SKYLIGHT_ENABLE_SOURCE_LOCATIONS=false appears to mimic our legacy 4.3.2 baseline. So something definitely needs optimizing with that feature set.

Additionally, the irony of a performance monitoring tool tanking performance is not lost on us. A +100% latency increase from a tool that we use to help reduce latency leaves our confidence in Skylight a bit shaken. 😢

[zvkemp]

Hi @colbymelvin, thank you for the report. Can you please let us know what version(s) of Ruby, Rails (if applicable), and the web server you are running?

@booleanbetrayal if your comment refers to a different app, could you please provide the same?

Lastly, I believe the comment in the spec file was left in error; Source Locations is a new feature, but we no longer consider it to be experimental.

[booleanbetrayal]

@zvkemp - Same org / app. We are running Rails 6.1.3 with Puma 5.2.2. I'm hesitant to disclose much more about our stack publicly, but can provide middleware, etc privately.

Edit: Also, Ruby 2.7.2p137

[zvkemp]

@booleanbetrayal you can email us directly at [email protected], or you can use the communicator widget in the Skylight UI. Please also send us your Skylight config, and as much of your Gemfile.lock/middleware stack as is easy enough to share. Some general information about the deployment infrastructure may also be helpful. Thanks in advance!