Add meta_struct and stack trace collection #4269

vpellan · 2025-01-09T13:38:33Z

What does this PR do?

This PR adds meta_struct and stack trace collection support

Motivation:

Stack trace collection is required for exploit prevention. meta_struct is required for stack trace collection

Change log entry

None.

Additional Notes:

How to test the change?

datadog-datadog-prod-us1 · 2025-01-09T13:53:16Z

Datadog Report

Branch report: vpellan/stack-trace-collection
Commit report: a6ab36f
Test service: dd-trace-rb

❌ 9 Failed (0 Known Flaky), 21946 Passed, 1476 Skipped, 5m 27.74s Total Time

❌ Failed Tests (9)

This report shows up to 5 failed tests.

Datadog::AppSec::ActionsHandler.handle calls both generate_stack and block_request when both are present - rspec - Details

Expand for error

 undefined method \`trace' for nil:NilClass
 Did you mean?  trap
 
 Failure/Error: service_entry_operation = context.trace || context.span
 
 NoMethodError:
   undefined method \`trace' for nil:NilClass
   Did you mean?  trap
 ./lib/datadog/appsec/stack_trace.rb:82:in \`add_stack_trace'
 ./lib/datadog/appsec/actions_handler.rb:23:in \`generate_stack'
 ...

Datadog::AppSec::ActionsHandler.handle calls both generate_stack and generate_schema when both are present - rspec - Details

Expand for error

 undefined method \`trace' for nil:NilClass
 Did you mean?  trap
 
 Failure/Error: service_entry_operation = context.trace || context.span
 
 NoMethodError:
   undefined method \`trace' for nil:NilClass
   Did you mean?  trap
 ./lib/datadog/appsec/stack_trace.rb:82:in \`add_stack_trace'
 ./lib/datadog/appsec/actions_handler.rb:23:in \`generate_stack'
 ...

Datadog::AppSec::ActionsHandler.handle calls both generate_stack and redirect_request when both are present - rspec - Details

Expand for error

 undefined method \`trace' for nil:NilClass
 Did you mean?  trap
 
 Failure/Error: service_entry_operation = context.trace || context.span
 
 NoMethodError:
   undefined method \`trace' for nil:NilClass
   Did you mean?  trap
 ./lib/datadog/appsec/stack_trace.rb:82:in \`add_stack_trace'
 ./lib/datadog/appsec/actions_handler.rb:23:in \`generate_stack'
 ...

Datadog::AppSec::ActionsHandler.handle calls generate_stack with action parameters - rspec - Details

Expand for error

 undefined method \`trace' for nil:NilClass
 Did you mean?  trap
 
 Failure/Error: service_entry_operation = context.trace || context.span
 
 NoMethodError:
   undefined method \`trace' for nil:NilClass
   Did you mean?  trap
 ./lib/datadog/appsec/stack_trace.rb:82:in \`add_stack_trace'
 ./lib/datadog/appsec/actions_handler.rb:23:in \`generate_stack'
 ...

Datadog::Tracing::Span#to_hash is expected to eq {:error=>0, :meta=>{}, :metrics=>{}, :name=>"my.span", :parent_id=>0, :resource=>"my.span", :service=>nil, :span_id=>34, :span_links=>[], :trace_id=>12, :type=>nil} - rspec - Details

Expand for error

 expected: {:error=>0, :meta=>{}, :metrics=>{}, :name=>"my.span", :parent_id=>0, :resource=>"my.span", :service=>nil, :span_id=>34, :span_links=>[], :trace_id=>12, :type=>nil}
      got: {:error=>0, :meta=>{}, :meta_struct=>{}, :metrics=>{}, :name=>"my.span", :parent_id=>0, :resource=>"my.span", :service=>nil, :span_id=>34, :span_links=>[], :trace_id=>12, :type=>nil}
 
 (compared using ==)
 
 Diff:
 @@ -1,5 +1,6 @@
  :error => 0,
  :meta => {},
 +:meta_struct => {},
 ...

pr-commenter · 2025-01-09T14:00:47Z

Benchmarks

Benchmark execution time: 2025-01-24 17:32:37

Comparing candidate commit 4b91c61 in PR branch vpellan/stack-trace-collection with baseline commit 4d51a86 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 31 metrics, 2 unstable metrics.

datadog-datadog-prod-us1 · 2025-01-21T15:20:02Z

spec/datadog/appsec/stack_trace_spec.rb

+
+        it 'creates a stack trace with 4 top frames' do
+          expect(dd_stack_trace.frames.size).to eq(4)
+          expect(dd_stack_trace.frames[0].text).to eq(stack_trace[0].to_s)


⚪ Code Quality Violation

Suggested change

expect(dd_stack_trace.frames[0].text).to eq(stack_trace[0].to_s)

expect(dd_stack_trace.frames[0].text).to eq(stack_trace.first.to_s)

Improve readability with first (...read more)

This rule encourages the use of first and last methods over array indexing to access the first and last elements of an array, respectively. The primary reason behind this rule is to improve code readability. Using first and last makes it immediately clear that you are accessing the first or last element of the array, which might not be immediately obvious with array indexing, especially for developers who are new to Ruby.

The use of these methods also helps to make your code more idiomatic, which is a crucial aspect of writing effective Ruby code. Idiomatic code is easier to read, understand, and maintain. It also tends to be more efficient, as idioms often reflect patterns that are optimized for the language.

To adhere to this rule, replace the use of array indexing with first or last methods when you want to access the first and last elements of an array. For instance, instead of arr[0] use arr.first and instead of arr[-1] use arr.last. However, note that this rule should be applied only when reading values. When modifying the first or last elements, array indexing should still be used. For example, arr[0] = 'new_value' and arr[-1] = 'new_value'.

datadog-datadog-prod-us1 · 2025-01-21T15:20:58Z

spec/datadog/appsec/stack_trace_spec.rb

+
+        it 'creates a stack trace with 4 frames, 1 top' do
+          expect(dd_stack_trace.frames.size).to eq(4)
+          expect(dd_stack_trace.frames[0].text).to eq(stack_trace[0].to_s)


⚪ Code Quality Violation

Suggested change

expect(dd_stack_trace.frames[0].text).to eq(stack_trace[0].to_s)

expect(dd_stack_trace.frames[0].text).to eq(stack_trace.first.to_s)

Improve readability with first (...read more)

This rule encourages the use of first and last methods over array indexing to access the first and last elements of an array, respectively. The primary reason behind this rule is to improve code readability. Using first and last makes it immediately clear that you are accessing the first or last element of the array, which might not be immediately obvious with array indexing, especially for developers who are new to Ruby.

The use of these methods also helps to make your code more idiomatic, which is a crucial aspect of writing effective Ruby code. Idiomatic code is easier to read, understand, and maintain. It also tends to be more efficient, as idioms often reflect patterns that are optimized for the language.

To adhere to this rule, replace the use of array indexing with first or last methods when you want to access the first and last elements of an array. For instance, instead of arr[0] use arr.first and instead of arr[-1] use arr.last. However, note that this rule should be applied only when reading values. When modifying the first or last elements, array indexing should still be used. For example, arr[0] = 'new_value' and arr[-1] = 'new_value'.

y9v · 2025-01-23T09:11:35Z

lib/datadog/appsec/contrib/graphql/reactive/multiplex.rb

@@ -35,7 +35,7 @@ def self.subscribe(engine, context)
                next unless result.match?

                yield result
-                throw(:block, true) unless result.actions.empty?
+                throw(:block, true) if result.actions.include?('block')


why are we changing this everywhere?

datadog-datadog-prod-us1 · 2025-01-23T16:55:47Z

Datadog Report

Branch report: vpellan/stack-trace-collection
Commit report: 4b91c61
Test service: dd-trace-rb

✅ 0 Failed, 22127 Passed, 1476 Skipped, 5m 17.04s Total Time

datadog-datadog-prod-us1 · 2025-01-24T17:09:44Z

spec/datadog/appsec/stack_trace_spec.rb

+
+        it 'creates a stack trace with 4 top frames' do
+          expect(collection.count).to eq(4)
+          expect(collection[0].text).to eq(frames[0].to_s)


⚪ Code Quality Violation

Suggested change

expect(collection[0].text).to eq(frames[0].to_s)

expect(collection.first.text).to eq(frames[0].to_s)

Improve readability with first (...read more)

This rule encourages the use of first and last methods over array indexing to access the first and last elements of an array, respectively. The primary reason behind this rule is to improve code readability. Using first and last makes it immediately clear that you are accessing the first or last element of the array, which might not be immediately obvious with array indexing, especially for developers who are new to Ruby.

The use of these methods also helps to make your code more idiomatic, which is a crucial aspect of writing effective Ruby code. Idiomatic code is easier to read, understand, and maintain. It also tends to be more efficient, as idioms often reflect patterns that are optimized for the language.

To adhere to this rule, replace the use of array indexing with first or last methods when you want to access the first and last elements of an array. For instance, instead of arr[0] use arr.first and instead of arr[-1] use arr.last. However, note that this rule should be applied only when reading values. When modifying the first or last elements, array indexing should still be used. For example, arr[0] = 'new_value' and arr[-1] = 'new_value'.

datadog-datadog-prod-us1 · 2025-01-24T17:09:44Z

spec/datadog/appsec/stack_trace_spec.rb

+
+        it 'creates a stack trace with 4 bottom frames' do
+          expect(collection.count).to eq(4)
+          expect(collection[0].text).to eq(frames[1].to_s)


⚪ Code Quality Violation

Suggested change

expect(collection[0].text).to eq(frames[1].to_s)

expect(collection.first.text).to eq(frames[1].to_s)

Improve readability with first (...read more)

This rule encourages the use of first and last methods over array indexing to access the first and last elements of an array, respectively. The primary reason behind this rule is to improve code readability. Using first and last makes it immediately clear that you are accessing the first or last element of the array, which might not be immediately obvious with array indexing, especially for developers who are new to Ruby.

The use of these methods also helps to make your code more idiomatic, which is a crucial aspect of writing effective Ruby code. Idiomatic code is easier to read, understand, and maintain. It also tends to be more efficient, as idioms often reflect patterns that are optimized for the language.

To adhere to this rule, replace the use of array indexing with first or last methods when you want to access the first and last elements of an array. For instance, instead of arr[0] use arr.first and instead of arr[-1] use arr.last. However, note that this rule should be applied only when reading values. When modifying the first or last elements, array indexing should still be used. For example, arr[0] = 'new_value' and arr[-1] = 'new_value'.

codecov-commenter · 2025-01-24T17:27:49Z

Codecov Report

Attention: Patch coverage is 92.52577% with 29 lines in your changes missing coverage. Please review.

Project coverage is 97.69%. Comparing base (4d51a86) to head (4b91c61).
Report is 56 commits behind head on master.

Files with missing lines	Patch %	Lines
lib/datadog/appsec/actions_handler.rb	27.77%	13 Missing ⚠️
lib/datadog/tracing/metadata/tagging.rb	45.00%	11 Missing ⚠️
lib/datadog/tracing/span_operation.rb	20.00%	4 Missing ⚠️
lib/datadog/appsec/stack_trace.rb	98.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4269      +/-   ##
==========================================
- Coverage   97.70%   97.69%   -0.02%     
==========================================
  Files        1359     1362       +3     
  Lines       82479    82864     +385     
  Branches     4198     4217      +19     
==========================================
+ Hits        80589    80954     +365     
- Misses       1890     1910      +20

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions bot added integrations Involves tracing integrations appsec Application Security monitoring product tracing labels Jan 9, 2025

vpellan force-pushed the vpellan/stack-trace-collection branch from f79044e to 77e9969 Compare January 14, 2025 15:40

datadog-datadog-prod-us1 bot reviewed Jan 21, 2025

View reviewed changes

vpellan added 17 commits January 21, 2025 16:43

Add tracing/span rbs typing

1f2c489

Add meta_struct methods

4ebb09a

Added stack trace collection settings

1fbef16

Add stack trace class

7c628b6

Add StackTrace class methods from_waf_result and include_stack_trace?

79d0575

WIP: Add meta_struct and stack traces support

0f82031

Lint

25cb3ab

fix stack traces size and setting

4d518f7

Fix id and frame initialization of stacktrace

7959669

Add DD_APPSEC_MAX_STACK_TRACE_DEPTH_TOP_PERCENT and max_depth support

b764ad4

Fix ASCII-8BIT encoding

e370b79

Use encode instead of force_encoding

a850808

Fix stack trace causing request blocking

1692579

Fix naming, float for top frames, and nil id

ead5e2f

Fix max_depth and max_collect behaviour when set to 0

6bf6a44

Add specs for StackTrace configuration and instances

76014d5

Fix stacktrace rbs after rebase

255e242

vpellan force-pushed the vpellan/stack-trace-collection branch from 5d75fc3 to 255e242 Compare January 21, 2025 16:17

vpellan added 2 commits January 22, 2025 10:01

Move "from_waf_result" to private methods

b103ed5

Add fStackTrace::Frame specs

18e79df

y9v reviewed Jan 23, 2025

View reviewed changes

Revert condition of throwing (unused code except in tests)

a6ab36f

Rework stacktrace collection architecture

f847fac

vpellan added 3 commits January 24, 2025 18:07

fix error when frames is nil and add []

0ac9467

fix stack_trace spec for new architecture

247e506

add meta_struct in span and serialization tests

4b91c61

datadog-datadog-prod-us1 bot reviewed Jan 24, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add meta_struct and stack trace collection #4269

Add meta_struct and stack trace collection #4269

vpellan commented Jan 9, 2025

datadog-datadog-prod-us1 bot commented Jan 9, 2025 •

edited

Loading

pr-commenter bot commented Jan 9, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot Jan 21, 2025

datadog-datadog-prod-us1 bot Jan 21, 2025

y9v Jan 23, 2025

datadog-datadog-prod-us1 bot commented Jan 23, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot Jan 24, 2025

datadog-datadog-prod-us1 bot Jan 24, 2025

codecov-commenter commented Jan 24, 2025

	expect(dd_stack_trace.frames[0].text).to eq(stack_trace[0].to_s)
	expect(dd_stack_trace.frames[0].text).to eq(stack_trace.first.to_s)

	expect(collection[0].text).to eq(frames[0].to_s)
	expect(collection.first.text).to eq(frames[0].to_s)

Add meta_struct and stack trace collection #4269

Are you sure you want to change the base?

Add meta_struct and stack trace collection #4269

Conversation

vpellan commented Jan 9, 2025

datadog-datadog-prod-us1 bot commented Jan 9, 2025 • edited Loading

Datadog Report

❌ Failed Tests (9)

pr-commenter bot commented Jan 9, 2025 • edited Loading

Benchmarks

datadog-datadog-prod-us1 bot Jan 21, 2025

Choose a reason for hiding this comment

⚪ Code Quality Violation

datadog-datadog-prod-us1 bot Jan 21, 2025

Choose a reason for hiding this comment

⚪ Code Quality Violation

y9v Jan 23, 2025

Choose a reason for hiding this comment

datadog-datadog-prod-us1 bot commented Jan 23, 2025 • edited Loading

Datadog Report

datadog-datadog-prod-us1 bot Jan 24, 2025

Choose a reason for hiding this comment

⚪ Code Quality Violation

datadog-datadog-prod-us1 bot Jan 24, 2025

Choose a reason for hiding this comment

⚪ Code Quality Violation

codecov-commenter commented Jan 24, 2025

Codecov Report

datadog-datadog-prod-us1 bot commented Jan 9, 2025 •

edited

Loading

pr-commenter bot commented Jan 9, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot commented Jan 23, 2025 •

edited

Loading