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.

Sign up for GitHub

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

Jump to bottom

Handle NameError during profiler initialization #1552

Merged
merged 2 commits into from
Jun 10, 2021
Merged

Handle NameError during profiler initialization #1552

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f5776d1
Handle NameError during profiler initialization
ivoanjo Jun 9, 2021
6f2df50
Add extra TODO to clarify when the weird check can be removed
ivoanjo Jun 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions lib/ddtrace/configuration/components.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,35 @@ def build_tracer(settings, agent_settings)
def build_profiler(settings, agent_settings)
return unless Datadog::Profiling.supported? && settings.profiling.enabled

unless defined?(Datadog::Profiling::Tasks::Setup)
# In #1545 a user reported a NameError due to this constant being uninitialized
# I've documented my suspicion on why that happened in
# https://github.com/DataDog/dd-trace-rb/issues/1545#issuecomment-856049025
#
# > Thanks for the info! It seems to feed into my theory: there's two moments in the code where we check if
# > profiler is "supported": 1) when loading ddtrace (inside preload) and 2) when starting the profile
# > after Datadog.configure gets run.
# > The problem is that the code assumes that both checks 1) and 2) will always reach the same conclusion:
# > either profiler is supported, or profiler is not supported.
# > In the problematic case, it looks like in your case check 1 decides that profiler is not
# > supported => doesn't load it, and then check 2 decides that it is => assumes it is loaded and tries to
# > start it.
#
# I was never able to validate if this was the issue or why exactly .supported? would change its mind BUT
# just in case it happens again, I've left this check which avoids breaking the user's application AND
# would instead direct them to report it to us instead, so that we can investigate what's wrong.
ericmustin marked this conversation as resolved.
Show resolved Hide resolved
#
# TODO: As of June 2021, most checks in .supported? are related to the google-protobuf gem; so it's
# very likely that it was the origin of the issue we saw. Thus, if, as planned we end up moving away from
# protobuf OR enough time has passed and no users saw the issue again, we can remove this check altogether.
Datadog.logger.error(
'Profiling was marked as supported and enabled, but setup task was not loaded properly. ' \
'Please report this at https://github.com/DataDog/dd-trace-rb/blob/master/CONTRIBUTING.md#found-a-bug'
)

return
end

# Load extensions needed to support some of the Profiling features
Datadog::Profiling::Tasks::Setup.new.run

Expand Down