nil access when enabling ASM without enabling tracing #3564
Comments
|
Thanks for the report and sorry for the issue.
It's not supposed to be so! AppSec should not crash when Tracing is disabled, it should merely not leverage spans and traces. We even have specs for that. Unfortunately it seems that the Devise integration does not account for that. We'll track this internally and be looking to fix this oversight. Your workaround is a good one.
A very interesting use case! Would you be able to elaborate on what exactly you mean by "dynamically per host" as well as "later enabled at runtime by looking at some AWS tags"? Especially, when in the app lifecycle (e.g at the Rails initializer stage?) is Tracing being enabled? |
Nice
We have a fleet of AWS servers, and only some of them are tagged on AWS to report to Datadog. As we add/remove servers, the servers are retagged to keep a specific number of reporting servers of each class (web/background jobs). In rails, there is a middleware that periodically checks if the current server is tagged or not, and enables/disables Datadog tracing (and now AppSec). I suppose Datadog maybe now has a configuration for this, but this was added long time ago and it's working for us fine. |
Is this a Rack middleware? If so this does the configuration via |
Yes. We use unicorn, so it's enabled/disabled just at the beginning of a request, and no other requests are running. |
|
OK you got me worried a moment 😅. If there were any concurrency there would have been problems as Thanks for the details, we'll keep that kind of use case in mind for the future. |
Exactly, at the very top
Thanks to you! |
|
We are using ddtrace 1.17 and devise 4.8.1 with the following initializer file in our rails app and getting the same error service_name = "companyx-admin"
datadog_tracing_enabled = ENV.fetch("DATADOG_TRACING_ENABLED", "false") == "true"
datadog_agent_host = if datadog_tracing_enabled
ENV.fetch("DATADOG_AGENT_HOST") do
Timeout.timeout(3) { Net::HTTP.get(URI.parse(ENV["AWS_METADATA_IP_URL"])) }
end
end
Datadog.configure do |c|
c.tracing.enabled = datadog_tracing_enabled
c.diagnostics.startup_logs.enabled = datadog_tracing_enabled
c.telemetry.enabled = false
c.agent.host = datadog_agent_host
c.env = ENV["RAILS_ENV"]
c.tags = {env: ENV["RAILS_ENV"]}
if datadog_tracing_enabled
# when Rails instrumentation is enabled,
# logs will be injected with trace_id and span_id,
# making them more verbose and harder to read
c.tracing.instrument :rails, service_name: service_name, log_injection: true
end
c.tracing.instrument :active_record, service_name: "#{service_name}-postgres", orm_service_name: "#{service_name}-activerecord"
c.tracing.instrument :sidekiq, service_name: "#{service_name}-sidekiq", client_service_name: "#{service_name}-sidekiq-client"
c.tracing.instrument :redis, service_name: "#{service_name}-redis"
c.tracing.instrument :faraday, service_name: "#{service_name}-faraday"
end
Datadog::Statsd.new(datadog_agent_host, 8125, {namespace: "foo"})Disabling appsec by setting DD_APPSEC_ENABLED=false made the error stop. |
|
@glaucocustodio in your situation, instead of But I understand that you may want to separately toggle appsec and tracing. Tangentially, I'm wondering why this block is not conditioned by IIUC when tracing is disabled this may still instrument but do nothing? Anyway, I'll try to come up with a fix shortly. |
|
yeah indeed the block could be nested in the condition.. here is the backtrack of the error, it might help you |
|
We've merged a fix to The fix should probably be backported to |
Background: we enable
tracingdynamically per host, so it's disabled by default and later enabled at runtime by looking at some AWS tags.We are trying ASM, so we just pasted the recommended config:
However, it seems that ASM has a strong dependency over tracing, and one can't be enabled without enabling the other.
Current behaviour
We get an
undefined method 'set_tag' for nil:NilClassexception onlib/datadog/appsec/contrib/devise/tracking.rb:34when logging in.Expected behaviour
No errors, to disable ASM if tracing is disabled, or some exception at configuration time. Not sure what should be the behaviour.
To bypass the error, we just disabled
appsecon static config and enabled it dynamically along withtracing.Steps to reproduce
How does
ddtracehelp you?Environment
Datadog.configure ...):The text was updated successfully, but these errors were encountered: