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

[APM] add sanitize_field_names & transaction_ignore_urls vars to Node.js agent remote config #85655

Merged
merged 2 commits into from
Dec 11, 2020

Conversation

trentm
Copy link
Member

@trentm trentm commented Dec 10, 2020

Summary

This adds the Node.js agent to the include list for two APM agent central config vars:

Checklist

For maintainers

Kibana unit tests

Running the relevant test file passed:

[x-pack]% node scripts/jest.js plugins/apm/common/agent_configuration/setting_definitions/index.test.ts
Running Jest with --config /Users/trentm/tm/kibana/x-pack/jest.config.js
jest-haste-map: duplicate manual mock found: index
  The following files share their name; please delete one of them:
    * <rootDir>/x-pack/plugins/enterprise_search/server/__mocks__/index.ts
    * <rootDir>/x-pack/plugins/enterprise_search/common/__mocks__/index.ts
...
jest-haste-map: duplicate manual mock found: index
  The following files share their name; please delete one of them:
    * <rootDir>/x-pack/plugins/security_solution/public/common/lib/kibana/__mocks__/index.ts
    * <rootDir>/x-pack/plugins/security_solution/public/overview/components/events_by_dataset/__mocks__/index.tsx

  PASS  plugins/apm/common/agent_configuration/setting_definitions/index.test.ts
  filterByAgent
    when `excludeAgents` is dotnet and nodejs
      ✓ should not include dotnet (1 ms)
      ✓ should include go
    when `includeAgents` is dotnet and nodejs
      ✓ should not include go (1 ms)
      ✓ should include dotnet (1 ms)
    options per agent
      ✓ go (1 ms)
      ✓ java (1 ms)
      ✓ js-base
      ✓ rum-js
      ✓ nodejs
      ✓ python
      ✓ dotnet (1 ms)
      ✓ ruby (1 ms)
      ✓ "All" services (no agent name) (1 ms)
  settingDefinitions
    ✓ should have correct default values (5 ms)

Test Suites: 1 passed, 1 total
Tests:       14 passed, 14 total
Snapshots:   1 passed, 1 total
Time:        2.239 s, estimated 3 s
Ran all test suites matching /plugins\/apm\/common\/agent_configuration\/setting_definitions\/index.test.ts/i.

End to end test

Creating an Agent settings configuration now includes those two vars as expected:

Screen Shot 2020-12-10 at 1 30 01 PM

I was able to observe my running play "esapp" updating its log level to "trace" (required to see the following log output):

% node esapp.js
...
Remote config failure. Unsupported config names: sanitize_field_names
Remote config success. Updated captureBody: off
Remote config success. Updated logLevel: trace
Remote config success. Updated transactionIgnoreUrls: /ping,/metrics,/ol*
Remote config success. Updated transactionSampleRate: 1
Remote config success. Updated transactionIgnoreUrlRegExp: /^\/ping$/i,/^\/metrics$/i,/^\/ol.*$/i
...

The "Unsupported config names: sanitize_field_names" is because that is coming in elastic/apm-agent-nodejs#1898 (currently in code review).
/cc @astorm

@trentm trentm added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Dec 10, 2020
@trentm trentm requested a review from sorenlouv December 10, 2020 22:07
@trentm trentm requested a review from a team as a code owner December 10, 2020 22:07
@trentm trentm self-assigned this Dec 10, 2020
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Dec 10, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@smith
Copy link
Contributor

smith commented Dec 10, 2020

@trentm looks like Jest failed because of a timeout. I can tell it to retest.

@smith
Copy link
Contributor

smith commented Dec 10, 2020

retest

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 5.4MB 5.4MB +18.0B

Distributable file count

id before after diff
default 46985 47745 +760

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@trentm trentm merged commit 2ea0816 into elastic:master Dec 11, 2020
@trentm trentm deleted the trentm-more-apm-agent-nodejs-config branch December 11, 2020 00:41
@trentm
Copy link
Member Author

trentm commented Dec 11, 2020

looks like Jest failed because of a timeout. I can tell it to retest.

Thanks, @smith!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v7.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants