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.

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

Test: Allow FakeAgent to respond to remote config requests #4626

Merged
merged 8 commits into from
Aug 28, 2024

Conversation

watson
Copy link
Collaborator

@watson watson commented Aug 26, 2024

Allow integration tests to mock the remote config abilities of the agent.

This is done by first setting up a new remote config "file" using the new FakeAgent#addRemoteConfig method before running the main part of the integration test. This primes the fake agent to respond with this config for http requests to the /v0.7/config endpoint.

Note to reviewers

This ability is needed by the integration tests in an upcoming Dynamic Instrumentation PR. I've separated it into its own PR to make the review easier.

I have not updated any of the existing tests to make uses of this ability. So this new ability of the FakeAgent class isn't actually used by any of our existing code in this PR. Let me know if there's an obvious place where we could make use of this so that I can update the PR to do so.

To make the review process easier, I've split this PR into two commits:

  1. 73cf5d7: This commit contains the actual code changes and is a good starting point for easier diffing.
  2. bc05f98: This commit contains only some refactoring to move code around to easier maintenance.

Usage example

Here's an example of how we'd use the new FakeAgent#addRemoteConfig method to register a Remote Config file for use in Dynamic Instrumentation:

agent.addRemoteConfig({
  product: 'LIVE_DEBUGGING',
  id: 'foobar',
  config: {
    id: 'foobar,
    version: 0,
    type: 'LOG_PROBE',
    language: 'javascript',
    where: { sourceFile: 'index.js', lines: ['15'] },
    tags: [],
    template: 'Hello World!',
    segments: [{ str: 'Hello World!' }],
    captureSnapshot: false,
    capture: { maxReferenceDepth: 3 },
    sampling: { snapshotsPerSecond: 5000 },
    evaluateAt: 'EXIT'
  }
})

@watson watson self-assigned this Aug 26, 2024
@watson watson requested a review from a team as a code owner August 26, 2024 11:20
Copy link

github-actions bot commented Aug 26, 2024

Overall package size

Self size: 6.99 MB
Deduped: 58.21 MB
No deduping: 58.49 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/native-appsec | 8.0.1 | 15.59 MB | 15.6 MB | | @datadog/native-iast-taint-tracking | 3.1.0 | 12.27 MB | 12.28 MB | | @datadog/pprof | 5.3.0 | 9.85 MB | 10.22 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.4.1 | 2.14 MB | 2.23 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 2.0.0 | 898.77 kB | 1.3 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.8.1 | 71.67 kB | 785.15 kB | | msgpack-lite | 0.1.26 | 201.16 kB | 281.59 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | lru-cache | 7.14.0 | 74.95 kB | 74.95 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | int64-buffer | 0.1.10 | 49.18 kB | 49.18 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | path-to-regexp | 0.1.7 | 6.78 kB | 6.78 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

Copy link

codecov bot commented Aug 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.65%. Comparing base (ba0442f) to head (f9cdc04).
Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4626       +/-   ##
===========================================
+ Coverage   68.65%   90.65%   +22.00%     
===========================================
  Files         252      123      -129     
  Lines       11047     4420     -6627     
  Branches       33       33               
===========================================
- Hits         7584     4007     -3577     
+ Misses       3463      413     -3050     

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

@watson watson force-pushed the watson/DEBUG-2532/fake-agent branch 3 times, most recently from adbb6e2 to a722990 Compare August 26, 2024 11:39
@pr-commenter
Copy link

pr-commenter bot commented Aug 26, 2024

Benchmarks

Benchmark execution time: 2024-08-28 14:22:40

Comparing candidate commit 9fe4ffb in PR branch watson/DEBUG-2532/fake-agent with baseline commit ba0442f in branch master.

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

@watson watson force-pushed the watson/DEBUG-2532/fake-agent branch 2 times, most recently from a9802cd to 9a3528a Compare August 26, 2024 14:42
@watson watson force-pushed the watson/DEBUG-2532/fake-agent branch from 9a3528a to d15767b Compare August 26, 2024 15:07
Copy link
Collaborator Author

watson commented Aug 26, 2024

@watson watson force-pushed the watson/DEBUG-2532/fake-agent branch from d15767b to 48ebf08 Compare August 26, 2024 15:12
@simon-id
Copy link
Member

I see there is no mention of RC capabilities. The client.capabilities field is a bitflag number encoded in base64 where each bit tells the backend that this client supports a specific feature. It's not technically required for the RC client to work, but we assume that the backend will send only what we support. I guess we can just assume the tests won't ever send data that we don't support 👍

@watson watson force-pushed the watson/DEBUG-2532/fake-agent branch from 48ebf08 to e36ae29 Compare August 27, 2024 11:37
@watson
Copy link
Collaborator Author

watson commented Aug 27, 2024

I guess we can just assume the tests won't ever send data that we don't support 👍

That was my idea, which is why I didn't bother validating the capabilities bitfield 🤷

@watson watson force-pushed the watson/DEBUG-2532/fake-agent branch from e36ae29 to c0243f8 Compare August 27, 2024 11:44
Copy link
Collaborator

@bengl bengl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the file name change, that's all.

integration-tests/helpers/fake_agent.js Outdated Show resolved Hide resolved
@watson watson force-pushed the watson/DEBUG-2532/fake-agent branch from c0243f8 to cb408e0 Compare August 27, 2024 16:21
@watson watson requested a review from a team as a code owner August 27, 2024 16:21
@watson watson force-pushed the watson/DEBUG-2532/fake-agent branch from cb408e0 to 899e115 Compare August 27, 2024 16:38
@watson watson requested review from bengl and simon-id August 27, 2024 16:40
integration-tests/helpers/fake-agent.js Outdated Show resolved Hide resolved
@watson watson force-pushed the watson/DEBUG-2532/fake-agent branch from 899e115 to bc05f98 Compare August 27, 2024 16:51
@watson watson requested a review from bengl August 27, 2024 16:53
@watson
Copy link
Collaborator Author

watson commented Aug 28, 2024

FYI, you can see this new capability of the FakeAgent in use in this upcoming PR: #4492 (look for the file integration-tests/debugger/index.spec.js)

Copy link
Member

@simon-id simon-id left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say let's remove all the superfluous fields. RC is complicated enough as it is haha. Let's keep the futurproofing for the futur, and focus on what we need to work first.
Lots of coments here sorry!

integration-tests/helpers.js Outdated Show resolved Hide resolved
integration-tests/helpers.js Outdated Show resolved Hide resolved
integration-tests/helpers/fake-agent.js Show resolved Hide resolved
integration-tests/helpers/fake-agent.js Outdated Show resolved Hide resolved
integration-tests/helpers/fake-agent.js Outdated Show resolved Hide resolved
integration-tests/helpers/fake-agent.js Show resolved Hide resolved
integration-tests/helpers/fake-agent.js Outdated Show resolved Hide resolved
integration-tests/helpers/fake-agent.js Outdated Show resolved Hide resolved
integration-tests/helpers/fake-agent.js Outdated Show resolved Hide resolved
watson added 6 commits August 28, 2024 10:59
This commit performs three operations:

1. Move `integration-tests/helpers.js` to
   `integration-tests/helpers/index.js`.
2. Move `FakeAgent` from inside `integration-tests/helpers/index.js` to
   its own file called `integration-tests/helpers/fake_agent.js`.
3. Move the setting up of the Express app inside the `FakeAgent.start`
   method to a private helper function.
@watson watson force-pushed the watson/DEBUG-2532/fake-agent branch from bc05f98 to f9cdc04 Compare August 28, 2024 12:26
@watson
Copy link
Collaborator Author

watson commented Aug 28, 2024

FYI, I've updated the #4492 PR with usage of FakeAgent#updateRemoveConfig and FakeAgent#removeRemoveConfig (look for the file integration-tests/debugger/index.spec.js)

@watson watson requested a review from simon-id August 28, 2024 13:27
@watson watson merged commit 198ead2 into master Aug 28, 2024
177 of 180 checks passed
@watson watson deleted the watson/DEBUG-2532/fake-agent branch August 28, 2024 14:45
bengl pushed a commit that referenced this pull request Aug 29, 2024
Allow integration tests to mock the remote config abilities of the agent.

This is done by first setting up a new remote config "file" using the new
`FakeAgent#addRemoteConfig` method before running the main part of the
integration test. This primes the fake agent to respond with this config for
http requests to the `/v0.7/config` endpoint.
@bengl bengl mentioned this pull request Aug 29, 2024
bengl pushed a commit that referenced this pull request Aug 29, 2024
Allow integration tests to mock the remote config abilities of the agent.

This is done by first setting up a new remote config "file" using the new
`FakeAgent#addRemoteConfig` method before running the main part of the
integration test. This primes the fake agent to respond with this config for
http requests to the `/v0.7/config` endpoint.
@bengl bengl mentioned this pull request Aug 29, 2024
bengl pushed a commit that referenced this pull request Aug 30, 2024
Allow integration tests to mock the remote config abilities of the agent.

This is done by first setting up a new remote config "file" using the new
`FakeAgent#addRemoteConfig` method before running the main part of the
integration test. This primes the fake agent to respond with this config for
http requests to the `/v0.7/config` endpoint.
bengl pushed a commit that referenced this pull request Aug 30, 2024
Allow integration tests to mock the remote config abilities of the agent.

This is done by first setting up a new remote config "file" using the new
`FakeAgent#addRemoteConfig` method before running the main part of the
integration test. This primes the fake agent to respond with this config for
http requests to the `/v0.7/config` endpoint.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants