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] Agent remote configuration: changes in Java property descriptions #62282

Merged
merged 6 commits into from
Apr 8, 2020

Conversation

eyalkoren
Copy link
Contributor

Some changes required for the Java property descriptions

@eyalkoren eyalkoren added bug Fixes for quality problems that affect the customer experience Team:APM All issues that need APM UI Team support labels Apr 2, 2020
@eyalkoren eyalkoren requested a review from a team as a code owner April 2, 2020 10:02
@elasticmachine
Copy link
Contributor

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

@bmorelli25 bmorelli25 added release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Apr 2, 2020
@bmorelli25
Copy link
Member

@elasticmachine merge upstream

@@ -175,7 +176,7 @@ export const javaSettings: RawSettingDefinition[] = [
'xpack.apm.agentConfig.profilingInferredSpansEnabled.description',
{
defaultMessage:
'Set to `true` to make the agent create spans for method executions based on async-profiler, a sampling aka statistical profiler. Due to the nature of how sampling profilers work, the duration of the inferred spans are not exact, but only estimations. The `profiling_inferred_spans_sampling_interval` lets you fine tune the trade-off between accuracy and overhead. The inferred spans are created after a profiling session has ended. This means there is a delay between the regular and the inferred spans being visible in the UI. This feature is not available on Windows'
'Set to `true` to make the agent create spans for method executions based on async-profiler, a sampling (aka statistical profiler). Due to the nature of how sampling profilers work, the duration of the inferred spans are not exact, but only estimations. The `profiling_inferred_spans_sampling_interval` lets you fine tune the trade-off between accuracy and overhead. The inferred spans are created after a profiling session has ended. This means there is a delay between the regular and the inferred spans being visible in the UI.\n\nNOTE: This feature is not available on Windows.'
Copy link
Member

Choose a reason for hiding this comment

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

I get what you're going for, but I don't the the parenthesis work in this case. Also, do the newlines actually render?

Here's a parenthesis suggestion:

Suggested change
'Set to `true` to make the agent create spans for method executions based on async-profiler, a sampling (aka statistical profiler). Due to the nature of how sampling profilers work, the duration of the inferred spans are not exact, but only estimations. The `profiling_inferred_spans_sampling_interval` lets you fine tune the trade-off between accuracy and overhead. The inferred spans are created after a profiling session has ended. This means there is a delay between the regular and the inferred spans being visible in the UI.\n\nNOTE: This feature is not available on Windows.'
'Set to `true` to make the agent create spans for method executions based on async-profiler, a sampling (or statistical) profiler. Due to the nature of how sampling profilers work, the duration of the inferred spans are not exact, but only estimations. The `profiling_inferred_spans_sampling_interval` lets you fine tune the trade-off between accuracy and overhead. The inferred spans are created after a profiling session has ended. This means there is a delay between the regular and the inferred spans being visible in the UI.\n\nNOTE: This feature is not available on Windows.'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's restore the former - a sampling aka statistical profiler.. I thought it would be better with parentheses, but seems you think it won't, so I will restore.
The main change I wanted here is the separation of the NOTE. I don't know about these newlines, I just saw what you did in other descriptions and did the same 🙂 . Since you found they do not render, I will remove...
So currently we have no formatting for these sections whatsoever?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, sounds good! Correct, there's unfortunately no formatting

Copy link
Member

@bmorelli25 bmorelli25 left a comment

Choose a reason for hiding this comment

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

One suggestion. Also, I noticed you added some of the newlines back in. Do those actually render? It didn't seem like they did when I worked on my previous PR.

@smith
Copy link
Contributor

smith commented Apr 7, 2020

retest

3 similar comments
@smith
Copy link
Contributor

smith commented Apr 7, 2020

retest

@smith
Copy link
Contributor

smith commented Apr 8, 2020

retest

@smith
Copy link
Contributor

smith commented Apr 8, 2020

retest

@smith
Copy link
Contributor

smith commented Apr 8, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@smith smith merged commit da51cc0 into master Apr 8, 2020
smith added a commit to smith/kibana that referenced this pull request Apr 8, 2020
…ns (elastic#62282)

* [APM] Agent remote configuration: changes in Java property descriptions

* Removing newlines

* Update snapshot

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
Co-authored-by: Nathan L Smith <smith@nlsmith.com>
smith added a commit to smith/kibana that referenced this pull request Apr 8, 2020
…ns (elastic#62282)

* [APM] Agent remote configuration: changes in Java property descriptions

* Removing newlines

* Update snapshot

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
Co-authored-by: Nathan L Smith <smith@nlsmith.com>
smith added a commit that referenced this pull request Apr 9, 2020
…ns (#62282) (#63066)

* [APM] Agent remote configuration: changes in Java property descriptions

* Removing newlines

* Update snapshot

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
Co-authored-by: Nathan L Smith <smith@nlsmith.com>

Co-authored-by: eyalkoren <41850454+eyalkoren@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Apr 9, 2020
…chore/put-all-xjson-together

* 'master' of github.com:elastic/kibana: (35 commits)
  [SIEM] [Detection Engine] Fixes bug when notification doesn't… (elastic#63013)
  [SIEM][Detection Engine] Fix rule notification critical bugs
  Add Error Exception Type Column (elastic#59596)
  [APM] Agent remote configuration: changes in Java property descriptions (elastic#62282)
  [Alerting] Displays warning when a permanent encryption key is missing and hides alerting UI appropriately (elastic#62772)
  FTR: add chromium-based Edge browser support (elastic#61684)
  [Ingest] Data source configuration validation UI (elastic#61180)
  restore empty_kibana after saved objects test (elastic#62951)
  Index pattern management plugin - src/legacy/core_plugins/management => new platform plugin (elastic#62594)
  Add basic StatusService (elastic#60335)
  [kbn/optimizer] link to kibanaReact/kibanaUtils plugins (elastic#62720)
  [APM] Service map - fixes layout issues for maps with no rum services (elastic#62887)
  Exclude disabled datasources and streams from agent config (elastic#62869)
  [Alerting] Fix validation support for nested IErrorObjects (elastic#62833)
  [Metrics UI] Invalidate non-count alerts which have no metrics (elastic#62837)
  Add --filter option to API docs script (elastic#62888)
  [Maps] fix attribution overflow with exit full screen button (elastic#62699)
  [Uptime]Alerting UI text in case filter is selected (elastic#62570)
  [Maps] Show create filter button for top-term tooltip property (elastic#62461)
  skip flaky suite (elastic#59030)
  ...

# Conflicts:
#	src/plugins/es_ui_shared/public/index.ts
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 9, 2020
* master: (40 commits)
  [ML] Functional transform tests - stabilize source selection (elastic#63087)
  add embed flag to saved object url as well (elastic#62926)
  [SIEM] [Detection Engine] Fixes bug when notification doesn't… (elastic#63013)
  [SIEM][Detection Engine] Fix rule notification critical bugs
  Add Error Exception Type Column (elastic#59596)
  [APM] Agent remote configuration: changes in Java property descriptions (elastic#62282)
  [Alerting] Displays warning when a permanent encryption key is missing and hides alerting UI appropriately (elastic#62772)
  FTR: add chromium-based Edge browser support (elastic#61684)
  [Ingest] Data source configuration validation UI (elastic#61180)
  restore empty_kibana after saved objects test (elastic#62951)
  Index pattern management plugin - src/legacy/core_plugins/management => new platform plugin (elastic#62594)
  Add basic StatusService (elastic#60335)
  [kbn/optimizer] link to kibanaReact/kibanaUtils plugins (elastic#62720)
  [APM] Service map - fixes layout issues for maps with no rum services (elastic#62887)
  Exclude disabled datasources and streams from agent config (elastic#62869)
  [Alerting] Fix validation support for nested IErrorObjects (elastic#62833)
  [Metrics UI] Invalidate non-count alerts which have no metrics (elastic#62837)
  Add --filter option to API docs script (elastic#62888)
  [Maps] fix attribution overflow with exit full screen button (elastic#62699)
  [Uptime]Alerting UI text in case filter is selected (elastic#62570)
  ...
smith added a commit that referenced this pull request Apr 9, 2020
…ns (#62282) (#63067)

* [APM] Agent remote configuration: changes in Java property descriptions

* Removing newlines

* Update snapshot

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
Co-authored-by: Nathan L Smith <smith@nlsmith.com>

Co-authored-by: eyalkoren <41850454+eyalkoren@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
@smith 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
@spalger spalger deleted the eyalkoren-suggested-changes-2 branch May 8, 2022 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:test-plan-done Pull request that was successfully tested during the test plan bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v7.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants