Skip to content

Commit

Permalink
Allow merging of sample data
Browse files Browse the repository at this point in the history
We've received requests in the past to merge sample data. Our API was
inconsistent in merging some sample data like tags, but not others
(params, headers, session data and custom data).

Support merging sample data for all sample data. All helpers now are
called `add_<data type>`. I've added aliases for the `set_<data type>`
methods.

This merging behavior is the new default, which is a breaking change.

I've added a SampleData helper class to handle the sample data being
merged. Array and Hashes are supported, same as in our extension. Other
types are not supported.

If the root value type switches between a Hash and Array between values
being merged, the new value is leading, discarding the previous value.

I've kept the behavior that if an application sets custom
params/headers/session data/custom data, we do not set it from our
integrations. This would merge it with the custom set data from the app,
which I don't know if we want to do that.
  • Loading branch information
tombruijn committed Aug 5, 2024
1 parent c12188e commit 272f18c
Show file tree
Hide file tree
Showing 25 changed files with 834 additions and 570 deletions.
31 changes: 31 additions & 0 deletions .changesets/merge-sample-data.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
bump: major
type: change
---

The transaction sample data is now merged by default. Previously, the sample data (except for tags) would be overwritten when an `Appsignal.set_*` helper was called.

```ruby
# Old behavior
Appsignal.set_params("param1" => "value")
Appsignal.set_params("param2" => "value")
# The parameters are:
# { "param2" => "value" }


# New behavior
Appsignal.add_params("param1" => "value")
Appsignal.add_params("param2" => "value")
# The parameters are:
# { "param1" => "value", "param2" => "value" }
```

New helpers have been added:

- `Appsignal.add_tags`
- `Appsignal.add_params`
- `Appsignal.add_session_data`
- `Appsignal.add_headers`
- `Appsignal.add_custom_data`

The old named helpers that start with `set_` will still work. They will also use the new merging behavior.
2 changes: 1 addition & 1 deletion benchmark.rake
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def monitor_transaction(transaction_id)
Appsignal::Transaction::HTTP_REQUEST
)
transaction.set_action("HomeController#show")
transaction.set_params(:id => 1)
transaction.add_params(:id => 1)

Appsignal.instrument("process_action.action_controller") do
Appsignal.instrument_sql(
Expand Down
1 change: 1 addition & 0 deletions lib/appsignal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ def collect_environment_metadata
end

require "appsignal/loaders"
require "appsignal/sample_data"
require "appsignal/environment"
require "appsignal/system"
require "appsignal/utils"
Expand Down
4 changes: 2 additions & 2 deletions lib/appsignal/demo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@ def add_demo_metadata_to(transaction)
end

def add_params_to(transaction)
transaction.set_params(
transaction.add_params(
"controller" => "demo",
"action" => "hello"
)
end

def add_headers_to(transaction)
transaction.set_headers(
transaction.add_headers(
"REMOTE_ADDR" => "127.0.0.1",
"REQUEST_METHOD" => "GET",
"SERVER_NAME" => "localhost",
Expand Down
Loading

0 comments on commit 272f18c

Please sign in to comment.