[APPSEC-8115] create AppSec::Monitor to subscribe to internal app sec events #2617
Conversation
github-actions
bot
added
appsec
GustavoCaso
force-pushed
the
appsec-pass-user-id-to-waf
branch
2 times, most recently
from
31a5be1 to
3c068ca
Compare
GustavoCaso
changed the title
GustavoCaso
force-pushed
the
appsec-pass-user-id-to-waf
branch
from
3c068ca to
bbe6a34
Compare
GustavoCaso
force-pushed
the
appsec-pass-user-id-to-waf
branch
2 times, most recently
from
e792d93 to
3fa663c
Compare
Codecov Report
@@ Coverage Diff @@
## master #2617 +/- ##
==========================================
- Coverage 98.01% 98.00% -0.02%
==========================================
Files 1148 1150 +2
Lines 62426 62620 +194
Branches 2808 2822 +14
==========================================
+ Hits 61189 61369 +180
- Misses 1237 1251 +14
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
GustavoCaso
changed the title
GustavoCaso
force-pushed
the
appsec-pass-user-id-to-waf
branch
from
c6541e1 to
341ea0f
Compare
lib/datadog/appsec.rb
Outdated
|
|
||
| # Internal appsec events monitor | ||
| require_relative 'appsec/monitor' | ||
| Datadog::AppSec::Monitor::Gateway::Watcher.watch |
There was a problem hiding this comment.
Probably this should be plugged into the AppSec component soon, notably to condition this behind AppSec being enabled.
GustavoCaso
force-pushed
the
appsec-pass-user-id-to-waf
branch
2 times, most recently
from
ff3bdba to
c8fc1ee
Compare
| ignore 'lib/datadog/appsec/monitor' | ||
| ignore 'lib/datadog/appsec/component.rb' |
There was a problem hiding this comment.
Were there any challenges in typing these that resulted in them being ignored?
lib/datadog/appsec/component.rb
Outdated
| @@ -19,12 +20,20 @@ def build_appsec_component(settings) | |||
|
|
|||
| def initialize(processor: Processor.new) | |||
| @processor = processor | |||
|
|
|||
| start_monitor | |||
There was a problem hiding this comment.
What happens when the component gets rebuilt? Are we at risk of having multiple watchers?
This might be good enough for now but to me it highlights that this may not be the correct place as this is instrumentation business and this way we're basically auto-instrumenting.
I'm having some idea, I need to chart it out.
There was a problem hiding this comment.
I thought about that.
Currently, the gateway, makes sure the same block is not registered twice.
| context = new_context | ||
| Processor.send(:active_context=, context) |
There was a problem hiding this comment.
What if active_context is not nil?
Should we allow a stack (i.e []#push on activation and []#pop on deactivation) or should we blow up and raise an exception?
There was a problem hiding this comment.
I think for simplicity know in which there is only a single processor, we could raise an exception
| @@ -7,6 +7,8 @@ target :appsec do | |||
| # check 'lib/datadog/kit' | |||
|
|
|||
| ignore 'lib/datadog/appsec/contrib' | |||
There was a problem hiding this comment.
Note that this is ignored because of the dependency on actual Sinatra/Rails/Rack constants and methods, which are not typed. We can relax that ignore by minimally typing just what we need in vendor/rbs.
…nd of the Request lifecycle
…to a separate method
…ntegrations. Move Rack::Reactive::SetUser to Monitor::Reactive::SetUser
…ithout having deactivate the previous one
GustavoCaso
force-pushed
the
appsec-pass-user-id-to-waf
branch
from
c8fc1ee to
e625807
Compare
GustavoCaso
force-pushed
the
appsec-pass-user-id-to-waf
branch
8 times, most recently
from
7008bd2 to
f566616
Compare
GustavoCaso
force-pushed
the
appsec-pass-user-id-to-waf
branch
from
f566616 to
4abbd89
Compare
| step_sorbet_type_checker: &step_sorbet_type_checker | ||
| run: | ||
| name: Run sorbet type checker | ||
| command: bundle exec rake typecheck |
There was a problem hiding this comment.
(Adding this for future reference) See #2641 for details on why we're removing Sorbet.
**What does this PR do?**: This PR is spiritually a revert of #1607, when we added the Sorbet typechecker to dd-trace-rb. It includes two commits: One where we remove all configuration and scaffolding surrounding Sorbet, and one where we remove all of the `# typed: ...` magic comments and `include Kernel` definitions added to make Sorbet happy. **Motivation**: As documented in #2641, the team has decided that the value vs pain equation for Sorbet has shifted in the past months, and thus that it was time to remove Sorbet. **Additional Notes**: Sorbet type checking in CI was actually removed earlier this week in #2617. **How to test the change?**: CI should still be green.
| @@ -63,6 +87,23 @@ def new_context | |||
| Context.new(self) | |||
| end | |||
|
|
|||
| def activate_context | |||
| existing_active_context = Processor.active_context | |||
| raise AlreadyActiveContextError if existing_active_context | |||
There was a problem hiding this comment.
This error is never rescued and breaks the app when it raises, is that intended behavior?
There was a problem hiding this comment.
There was a problem hiding this comment.
Couldn't figure out the root cause. I thought it was because we were using custom instrumentation before the appsec middleware kicked in, but even after removing all the ddtrace/ddstatsd interaction the error is still there.
I don't think the backtrace will be helpful, but here's an example:
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/appsec/processor.rb:90:in `activate_context'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/appsec/contrib/rack/request_middleware.rb:35:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/tracing/contrib/rack/middlewares.rb:70:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/sinatra-3.0.5/lib/sinatra/base.rb:2007:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/urlmap.rb:74:in `block in call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/urlmap.rb:58:in `each'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/urlmap.rb:58:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-parser-0.7.0/lib/rack/parser.rb:24:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-utf8_sanitizer-1.8.0/lib/rack/utf8_sanitizer.rb:28:in `call'",
"/home/wetransfer/frontend/lib/middleware/refuse_non_json_or_multipart_requests.rb:7:in `call'",
"/home/wetransfer/frontend/lib/middleware/json_rescue.rb:38:in `call_with_exception_handling'",
"/home/wetransfer/frontend/lib/middleware/json_rescue.rb:17:in `call'",
"/home/wetransfer/frontend/lib/middleware/x_frame_options_deny.rb:5:in `call'",
"/home/wetransfer/frontend/lib/api/v4.rb:41:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/routing/mapper.rb:20:in `block in <class:Constraints>'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/routing/mapper.rb:49:in `serve'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/journey/router.rb:50:in `block in serve'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/journey/router.rb:32:in `each'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/journey/router.rb:32:in `serve'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/routing/route_set.rb:842:in `call'",
"/home/wetransfer/frontend/config/initializers/redirect_invalid_subdomains.rb:22:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/sanitize_user_agent_header-0.1.0/lib/sanitize_user_agent_header.rb:21:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/deflater.rb:44:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activerecord-6.1.7.1/lib/active_record/middleware/database_selector.rb:53:in `block in call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activerecord-6.1.7.1/lib/active_record/middleware/database_selector/resolver.rb:71:in `block (2 levels) in write_to_primary'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activesupport-6.1.7.1/lib/active_support/notifications/instrumenter.rb:24:in `instrument'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activerecord-6.1.7.1/lib/active_record/middleware/database_selector/resolver.rb:70:in `block in write_to_primary'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activerecord-6.1.7.1/lib/active_record/connection_handling.rb:388:in `with_role_and_shard'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activerecord-6.1.7.1/lib/active_record/connection_handling.rb:175:in `connected_to'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activerecord-6.1.7.1/lib/active_record/middleware/database_selector/resolver.rb:69:in `write_to_primary'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activerecord-6.1.7.1/lib/active_record/middleware/database_selector/resolver.rb:44:in `write'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activerecord-6.1.7.1/lib/active_record/middleware/database_selector.rb:65:in `select_database'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activerecord-6.1.7.1/lib/active_record/middleware/database_selector.rb:52:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/tempfile_reaper.rb:15:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/etag.rb:27:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/conditional_get.rb:27:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/head.rb:12:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/http/permissions_policy.rb:22:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/http/content_security_policy.rb:19:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/strict_request_uri-1.0.3/lib/strict_request_uri.rb:36:in `call'",
"/home/wetransfer/frontend/config/initializers/set_amplitude_user_properties.rb:7:in `call'",
"/home/wetransfer/frontend/config/initializers/set_amplitude_device_id.rb:7:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/session/abstract/id.rb:266:in `context'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/session/abstract/id.rb:260:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/middleware/cookies.rb:697:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/middleware/callbacks.rb:27:in `block in call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activesupport-6.1.7.1/lib/active_support/callbacks.rb:98:in `run_callbacks'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/middleware/callbacks.rb:26:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/middleware/actionable_exceptions.rb:18:in `call'",
"/home/wetransfer/frontend/config/initializers/client_ip_injection.rb:46:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/tracing/contrib/rails/middlewares.rb:17:in `call'",
"/home/wetransfer/frontend/config/initializers/suppress_unacceptable_format.rb:8:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/middleware/debug_exceptions.rb:29:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/lograge-0.12.0/lib/lograge/rails_ext/rack/logger.rb:18:in `call_app'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/railties-6.1.7.1/lib/rails/rack/logger.rb:26:in `block in call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activesupport-6.1.7.1/lib/active_support/tagged_logging.rb:99:in `block in tagged'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activesupport-6.1.7.1/lib/active_support/tagged_logging.rb:37:in `tagged'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/activesupport-6.1.7.1/lib/active_support/tagged_logging.rb:99:in `tagged'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/railties-6.1.7.1/lib/rails/rack/logger.rb:26:in `call'",
"/home/wetransfer/frontend/config/initializers/quiet_assets.rb:6:in `call_with_quiet_assets'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/middleware/remote_ip.rb:81:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/request_store-1.5.1/lib/request_store/middleware.rb:19:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/middleware/request_id.rb:26:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/method_override.rb:24:in `call'",
"/home/wetransfer/frontend/config/initializers/host_ident.rb:18:in `call'",
"/home/wetransfer/frontend/config/initializers/i18n.rb:20:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/runtime.rb:22:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/middleware/executor.rb:14:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/middleware/static.rb:24:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/sendfile.rb:110:in `call'",
"/home/wetransfer/frontend/config/initializers/accept_header_fix.rb:7:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/actionpack-6.1.7.1/lib/action_dispatch/middleware/host_authorization.rb:148:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-cors-2.0.0/lib/rack/cors.rb:102:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/appsec/contrib/rack/request_middleware.rb:44:in `block (2 levels) in call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/appsec/instrumentation/gateway.rb:37:in `block in push'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/appsec/contrib/rack/gateway/watcher.rb:56:in `block in watch_request'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/appsec/instrumentation/gateway.rb:19:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/appsec/instrumentation/gateway.rb:43:in `block (2 levels) in push'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/appsec/instrumentation/gateway.rb:47:in `push'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/appsec/contrib/rack/request_middleware.rb:43:in `block in call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/appsec/contrib/rack/request_middleware.rb:42:in `catch'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/appsec/contrib/rack/request_middleware.rb:42:in `call'",
"/home/wetransfer/frontend/config/initializers/referer_tracking_middleware.rb:5:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.10.1/lib/datadog/tracing/contrib/rack/middlewares.rb:98:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/railties-6.1.7.1/lib/rails/engine.rb:539:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/urlmap.rb:74:in `block in call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/urlmap.rb:58:in `each'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/rack-2.2.6.4/lib/rack/urlmap.rb:58:in `call'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/unicorn-6.1.0/lib/unicorn/http_server.rb:634:in `process_client'",
"config/unicorn.rb:33:in `block (2 levels) in reload'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/unicorn-6.1.0/lib/unicorn/http_server.rb:739:in `worker_loop'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/unicorn-6.1.0/lib/unicorn/http_server.rb:547:in `spawn_missing_workers'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/unicorn-6.1.0/lib/unicorn/http_server.rb:143:in `start'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/gems/unicorn-6.1.0/bin/unicorn:128:in `<top (required)>'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/bin/unicorn:25:in `load'",
"/home/wetransfer/frontend/vendor/bundle/ruby/3.2.0/bin/unicorn:25:in `<top (required)>'",
"/usr/local/bundle/gems/bundler-2.2.33/lib/bundler/cli/exec.rb:58:in `load'",
"/usr/local/bundle/gems/bundler-2.2.33/lib/bundler/cli/exec.rb:58:in `kernel_load'",
"/usr/local/bundle/gems/bundler-2.2.33/lib/bundler/cli/exec.rb:23:in `run'",
"/usr/local/bundle/gems/bundler-2.2.33/lib/bundler/cli.rb:479:in `exec'",
"/usr/local/bundle/gems/bundler-2.2.33/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'",
"/usr/local/bundle/gems/bundler-2.2.33/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'",
"/usr/local/bundle/gems/bundler-2.2.33/lib/bundler/vendor/thor/lib/thor.rb:392:in `dispatch'",
"/usr/local/bundle/gems/bundler-2.2.33/lib/bundler/cli.rb:31:in `dispatch'",
"/usr/local/bundle/gems/bundler-2.2.33/lib/bundler/vendor/thor/lib/thor/base.rb:485:in `start'",
"/usr/local/bundle/gems/bundler-2.2.33/lib/bundler/cli.rb:25:in `start'",
"/usr/local/bundle/gems/bundler-2.2.33/exe/bundle:49:in `block in <top (required)>'",
"/usr/local/bundle/gems/bundler-2.2.33/lib/bundler/friendly_errors.rb:103:in `with_friendly_errors'",
"/usr/local/bundle/gems/bundler-2.2.33/exe/bundle:37:in `<top (required)>'",
"/usr/local/bundle/bin/bundle:25:in `load'",
"/usr/local/bundle/bin/bundle:25:in `<main>'"
Please let me know if there is a way to evade appsec errors, it breaks our app. If there is no mitigation, let me know if we need to open a ticket with the dd support instead.
Thanks 🙌
There was a problem hiding this comment.
@idanci, thanks for sharing the backtrace.
Could you share more information about your system?
From the backtrace, I see you are using dd-trace-rb 1.10.1
Is it a Sinatra app?
Could you share your Datadog.configure block?
There was a problem hiding this comment.
rails 6.1.7.1
ruby 3.2
ddtrace 1.10.1
dogstatsd-ruby 5.4.0
We use rails and mount several sinatra apps within rails. Those sinatra apps also have home custom middlewares on top of default stack.
require 'ddtrace/auto_instrument' unless Rails.env.test?
Datadog.configure do |c|
options = { service_name: 'frontend' }
c.service = options[:service_name]
c.tracing.enabled = true
if ENV['DD_PROFILING_ENABLED'].present?
c.profiling.enabled = true
c.profiling.advanced.force_enable_new_profiler = true
end
c.tracing.instrument :sinatra, options.merge(resource_script_names: true)
c.tracing.instrument :active_support, cache_service: options[:service_name]
c.tracing.instrument :active_record, options
c.tracing.instrument :rails, options unless Rails.env.test?
c.tracing.instrument :faraday, options
c.tracing.instrument :rest_client, options
c.tracing.instrument :mysql2, options
c.tracing.instrument :redis, options
c.tracing.instrument :active_job, options
c.tracing.instrument :rake, options.merge(tasks: %w[list of our tasks])
if Rails.env.production? # we disabled this for now, it breaks the app
require 'datadog/appsec'
c.appsec.enabled = true
c.appsec.instrument :rack
c.appsec.instrument :rails
c.appsec.instrument :sinatra
end
end
class DatadogCustomTraceProcessor
def call(trace)
trace.spans.each do |span|
next unless span.status == Datadog::Tracing::Metadata::Ext::Errors::STATUS
error_stack = span.get_tag('error.stack').dup
next if error_stack.blank?
span.set_tag('error.original_stack', error_stack)
span.set_tag('error.stack', backtrace_for(error_stack))
end
trace
end
private
def backtrace_for(error_stack)
cleaned_stack = backtrace_cleaner.clean(error_stack.split("\n\tfrom "))
cleaned_stack.map(&:strip).join("\n")
end
def backtrace_cleaner
@backtrace_cleaner ||= ActiveSupport::BacktraceCleaner.new.tap do |bc|
bc.add_filter { |line| line.gsub(Rails.root.to_s, 'frontend') } # strip the Rails.root prefix
bc.add_silencer { |line| /\/gems\/|vendor\/bundle|usr\/local/.match?(line) } # skip irrelevant lines
end
end
end
Datadog::Tracing.before_flush(DatadogCustomTraceProcessor.new)There was a problem hiding this comment.
While disabling sinatra fixes the error, enabling appsec even without sinatra on ddtrace 1.10.1 resulted sky rocketing latency 😬
Were there any reported regressions after 1.10? Can't find anything in issues
There was a problem hiding this comment.
Hey @idanci so sorry for the delay
Regarding:
What will be the best way to proceed? Should we revert to an older ddtrace version or keep the current one with sinatra appsec disabled and wait for the ddtrace update?
Version 1.10.1 has bug fixes on the Client IP resolution logic #2665
So reverting to an earlier version will fix the issue with raise AlreadyActiveContextError, meaning that the ClientIP reporting could be buggy.
Staying in 1.10.1 and removing c.appsec.instrument :sinatra would mean that app would be able to load, but the custom appsec events for the nested sinatra apps won't be checked. The events are checking the request body, and the URL path of the sinatra requests.
So is up to you which setup works best in your case.
Again I'm so sorry that we have affected the normal workflow for your application setup and appsec. We are going to make sure to have a fix on the next release that would allow you to use your current setup with nested apps.
As for:
While disabling sinatra fixes the error, enabling appsec even without sinatra on ddtrace 1.10.1 resulted sky rocketing latency 😬
Do you mind opening a GH issue so someone from the team can have a look and figure out what is going on?
Thank you
There was a problem hiding this comment.
We disabled appsec completely due to performance issues, will try to debug it when there's time.
I can open a GH issue, can you please advise what info to put there?
There are no datadog traces for the requests that failed because they were cut off on the client side due to timeouts. We can't spot the issue with ddtrace because ddtrace itself has an issue 😬 . The problem as well might be in our ddtrace config that is shared above, but nothing seems suspicious there.
Thanks for looking into it 🙌
There was a problem hiding this comment.
I can open a GH issue, can you please advise what info to put there?
Mentioning your current Datadog configuration, and mentioning what you experience when you upgraded from 1.10.0 to 1.10.1 would be enough.
We would look into our reliability environment to figure out what change could have caused that degradation between releases.
Thank you so much for your patience 🙌
There was a problem hiding this comment.
I created a GH issue to track this: #2722
Let's move any future discussion over there.
What does this PR do?
This PR introduces a new reactive component calledRack::Reactive::SetUser. The reactive component observes theusr-idkey. It uses that key to communicate with the waf.Update after talking with @lloeki
We decided to move the reactive
SetUsercomponent outside of thecontrib/racknamespace. This monitor is agnostic of Rack or any integration within app sec. For those types of events, we would like an internal monitor that would always be enabled whenappsecis required.This PR doesn't publish events yet to the
identity.set_userevent. That will be done in a future PR.The PR is divided into separate commits to make easier the review of each of them separately:
Motivation
In the future, we will publish an event when
Datadog::Kit::Identity.set_userfunction is being called from the customer code. Depending on the configureduser_iddeny list we would be able to block the request.