[APM] Agent remote configuration: general settings descriptions #62276

eyalkoren · 2020-04-02T09:22:19Z

Suggesting some changes for the general config options descriptions.

I removed log_level from here because it seems it doesn't fit at least the Java, Go and Node agents - see #61821, and it is already deactivated for most others, however this doesn't have to be included in this PR.

I removed `log_level` from here because it seems it doesn't fit at least the Java, Go and Node agents - see #61821, and it is already deactivated for most others (this doesn't have to be included in this PR though).

cauemarcondes · 2020-04-02T09:30:14Z

x-pack/plugins/apm/common/agent_configuration/setting_definitions/general_settings.ts

-  // LOG_LEVEL
-  {
-    key: 'log_level',
-    type: 'text',
-    defaultValue: 'info',
-    label: i18n.translate('xpack.apm.agentConfig.logLevel.label', {
-      defaultMessage: 'Log level'
-    }),
-    description: i18n.translate('xpack.apm.agentConfig.logLevel.description', {
-      defaultMessage: 'Sets the logging level for the agent'
-    }),
-    excludeAgents: ['js-base', 'rum-js', 'python']
-  },
-


@eyalkoren unless we decide to remove it from dotnet and ruby, we should keep it. @formgeist WDYT?

I don't know what is the cutoff to use in agent-dedicated config description vs. in this general_settings one. Otherwise it means increasing this excludeAgents to contain 5 agents.
It's really up to you, just a suggestion.

I have changed it to includeAgents instead of excludeAgents to make it easy to read.

includeAgents: ['dotnet', 'ruby']

But I'm not against removing it if any agent is going to use it. 😃

@cauemarcondes so you have both options now? Include and exclude? Isn't there an option to only show for one agent, I believe Java had a bunch that is Java-specific?

@formgeist no, I removed the excludeAgents and replaced it for includeAgents, so this option will be available for dotnet and ruby only. Java is the only one who has its own config file, it must be because of what you said (Java had a bunch that is Java-specific ).
My question here is if we can remove the log_level property from all agents., if yes we can keep what @eyalkoren did here.

We should keep the log_level option for ruby and dotnet since they have this active in their dynamic config options.

formgeist · 2020-04-02T11:35:45Z

@eyalkoren Will the copy changes to the descriptions also be updated in the Java documentation, since that's where these are lifted from?

eyalkoren · 2020-04-02T12:05:00Z

Will the copy changes to the descriptions also be updated in the Java documentation, since that's where these are lifted from?

Yes, see elastic/apm-agent-java#1119

cauemarcondes

LGTM 👍🏻

bmorelli25

Descriptions LGTM. I'm still not convinced that \n actually does anything though 🤷‍♂

cauemarcondes · 2020-04-03T07:12:59Z

@eyalkoren you can leave the log as it was.

{
    key: 'log_level',
    type: 'text',
    defaultValue: 'info',
    label: i18n.translate('xpack.apm.agentConfig.logLevel.label', {
      defaultMessage: 'Log level'
    }),
    description: i18n.translate('xpack.apm.agentConfig.logLevel.description', {
      defaultMessage: 'Sets the logging level for the agent'
    }),
    excludeAgents: ['js-base', 'rum-js', 'python']
  },

I've already changed it on my #62290

eyalkoren · 2020-04-05T07:07:50Z

log_level restored

smith · 2020-04-06T20:20:22Z

@elasticmachine merge upstream

elasticmachine · 2020-04-06T20:20:24Z

merge conflict between base and head

…ed-changes

kibanamachine · 2020-04-07T23:56:26Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 253ed54

History

💚 Build #38702 succeeded 883d6ca
💔 Build #38122 failed eac3f72
💔 Build #38074 failed 19afe68

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

…tic#62276) * Updates general remote config descriptions I removed `log_level` from here because it seems it doesn't fit at least the Java, Go and Node agents - see elastic#61821, and it is already deactivated for most others (this doesn't have to be included in this PR though). * Update general_settings.ts * Restore log_level definition * Remove extra spaces Co-authored-by: Nathan L Smith <smith@nlsmith.com>

…) (#62954) * Updates general remote config descriptions I removed `log_level` from here because it seems it doesn't fit at least the Java, Go and Node agents - see #61821, and it is already deactivated for most others (this doesn't have to be included in this PR though). * Update general_settings.ts * Restore log_level definition * Remove extra spaces Co-authored-by: Nathan L Smith <smith@nlsmith.com> Co-authored-by: eyalkoren <41850454+eyalkoren@users.noreply.github.com>

…) (#62955) * Updates general remote config descriptions I removed `log_level` from here because it seems it doesn't fit at least the Java, Go and Node agents - see #61821, and it is already deactivated for most others (this doesn't have to be included in this PR though). * Update general_settings.ts * Restore log_level definition * Remove extra spaces Co-authored-by: Nathan L Smith <smith@nlsmith.com> Co-authored-by: eyalkoren <41850454+eyalkoren@users.noreply.github.com>

Updates general remote config descriptions

19afe68

I removed `log_level` from here because it seems it doesn't fit at least the Java, Go and Node agents - see #61821, and it is already deactivated for most others (this doesn't have to be included in this PR though).

eyalkoren requested review from formgeist and bmorelli25 April 2, 2020 09:22

eyalkoren requested a review from a team as a code owner April 2, 2020 09:22

eyalkoren changed the title ~~Updates general remote config descriptions~~ [APM] Agent remote configuration: general settings descriptions Apr 2, 2020

cauemarcondes reviewed Apr 2, 2020

View reviewed changes

Update general_settings.ts

eac3f72

cauemarcondes added release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Apr 2, 2020

cauemarcondes approved these changes Apr 2, 2020

View reviewed changes

bmorelli25 approved these changes Apr 2, 2020

View reviewed changes

eyalkoren added 2 commits April 5, 2020 10:05

Restore log_level definition

60a1084

Remove extra spaces

883d6ca

smith mentioned this pull request Apr 7, 2020

[APM] Agent Configuration test scenarios #61694

Closed

Merge remote-tracking branch 'upstream/master' into eyalkoren-suggest…

253ed54

…ed-changes

smith merged commit 1af82c7 into master Apr 8, 2020

smith mentioned this pull request Apr 8, 2020

[7.x] [APM] Agent remote configuration: general settings descriptions (#62276) #62954

Merged

smith mentioned this pull request Apr 8, 2020

[7.7] [APM] Agent remote configuration: general settings descriptions (#62276) #62955

Merged

bmorelli25 deleted the eyalkoren-suggested-changes branch April 8, 2020 17:44

smith added apm-test-plan-7.7.0 apm:test-plan-done Pull request that was successfully tested during the test plan labels Apr 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] Agent remote configuration: general settings descriptions #62276

[APM] Agent remote configuration: general settings descriptions #62276

eyalkoren commented Apr 2, 2020

cauemarcondes Apr 2, 2020

eyalkoren Apr 2, 2020

cauemarcondes Apr 2, 2020

formgeist Apr 2, 2020

cauemarcondes Apr 2, 2020

formgeist Apr 2, 2020

eyalkoren Apr 2, 2020

formgeist commented Apr 2, 2020

eyalkoren commented Apr 2, 2020

cauemarcondes left a comment

bmorelli25 left a comment

cauemarcondes commented Apr 3, 2020

eyalkoren commented Apr 5, 2020

smith commented Apr 6, 2020

elasticmachine commented Apr 6, 2020

kibanamachine commented Apr 7, 2020

[APM] Agent remote configuration: general settings descriptions #62276

[APM] Agent remote configuration: general settings descriptions #62276

Conversation

eyalkoren commented Apr 2, 2020

cauemarcondes Apr 2, 2020

Choose a reason for hiding this comment

eyalkoren Apr 2, 2020

Choose a reason for hiding this comment

cauemarcondes Apr 2, 2020

Choose a reason for hiding this comment

formgeist Apr 2, 2020

Choose a reason for hiding this comment

cauemarcondes Apr 2, 2020

Choose a reason for hiding this comment

formgeist Apr 2, 2020

Choose a reason for hiding this comment

eyalkoren Apr 2, 2020

Choose a reason for hiding this comment

formgeist commented Apr 2, 2020

eyalkoren commented Apr 2, 2020

cauemarcondes left a comment

Choose a reason for hiding this comment

bmorelli25 left a comment

Choose a reason for hiding this comment

cauemarcondes commented Apr 3, 2020

eyalkoren commented Apr 5, 2020

smith commented Apr 6, 2020

elasticmachine commented Apr 6, 2020

kibanamachine commented Apr 7, 2020

💚 Build Succeeded

History