Skip to content

Conversation

@felixbarny
Copy link
Member

Update 404 handling

logger.info("Received new configuration from APM Server: <empty>");
}
config = Collections.emptyMap();
configurationRegistry.reloadDynamicConfigurationOptions();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this means a user can't cancel configuration (ie restore defaults) by just removing it, and is instead required to actively set everything to specified values?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I completely understand. The user can delete the central configuration in Kibana. The APM Server would then respond with a status code 200 and a {} body. That basically sets the config map to an empty map.

Copy link
Contributor

Choose a reason for hiding this comment

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

The comment says:

// means that there either is no configuration for this agent or...

Sorry, I don't remember the spec, but I read that as getting 404 in such case

Copy link
Member Author

Choose a reason for hiding this comment

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

The spec has recently been changed. The APM Server will now only return with 404 if it's pre 7.3: elastic/apm#118

Copy link
Member Author

Choose a reason for hiding this comment

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

removed the outdated comment

@codecov-io
Copy link

codecov-io commented Jul 23, 2019

Codecov Report

Merging #739 into master will increase coverage by 0.28%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #739      +/-   ##
============================================
+ Coverage     64.79%   65.07%   +0.28%     
+ Complexity      169       68     -101     
============================================
  Files           209      209              
  Lines          8925     9006      +81     
  Branches       1147     1161      +14     
============================================
+ Hits           5783     5861      +78     
- Misses         2788     2791       +3     
  Partials        354      354
Impacted Files Coverage Δ Complexity Δ
...nt/configuration/ApmServerConfigurationSource.java 92.59% <0%> (-0.27%) 0% <0%> (ø)
...lastic/apm/agent/report/ReporterConfiguration.java 100% <0%> (ø) 0% <0%> (ø) ⬇️
...agent/impl/stacktrace/StacktraceConfiguration.java 100% <0%> (ø) 0% <0%> (-3%) ⬇️
...tic/apm/agent/configuration/CoreConfiguration.java 97.24% <0%> (+0.29%) 0% <0%> (ø) ⬇️
...astic/apm/agent/impl/transaction/AbstractSpan.java 87.79% <0%> (+4.15%) 0% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01400f2...34795d1. Read the comment docs.

@felixbarny felixbarny force-pushed the remote-config-feature-flag branch 2 times, most recently from bc76da2 to c449c98 Compare July 24, 2019 06:59
@felixbarny felixbarny force-pushed the remote-config-feature-flag branch from c449c98 to 34795d1 Compare July 24, 2019 07:58
@felixbarny felixbarny merged commit 21f161f into elastic:master Jul 26, 2019
@felixbarny felixbarny deleted the remote-config-feature-flag branch July 26, 2019 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants