Actuator content negotiation #1438

TimHess · 2025-01-16T18:05:06Z

Description

Improves content negotiation, provides per-request support for determining version
Removes most v1/v2 remnants that aren't needed anymore.
Improves Spring alignment for Liveness, Readiness, Disk Space, /health v3

Now includes changes from #1445

Resolves #1335, Resolves #1367

Partially addresses #1440 (probes are still enabled by default, showDetails not investigated yet)

TODO:

docs
further alignment with Spring on probes and groups

Quality checklist

Your code complies with our Coding Style.
You've updated unit and/or integration tests for your change, where applicable.
You've updated documentation for your change, where applicable.
If your change affects other repositories, such as Documentation, Samples and/or MainSite, add linked PRs here.
There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.
You've added required license files and/or file headers (explaining where the code came from with proper attribution), where code is copied from StackOverflow, a blog, or OSS.

bart-vmware

Looks great, happy to see all the startup classes are gone.

Although we decided to change Steeltoe to not perform any content negotiation anymore, I'm concerned adding it later is going to be a breaking change. Therefore I opened #1445, which builds upon this PR.

src/Management/src/Endpoint/Actuators/Health/EndpointServiceCollectionExtensions.cs

src/Management/src/Endpoint/Actuators/Loggers/LoggersEndpointMiddleware.cs

src/Management/src/Endpoint/Actuators/ThreadDump/EndpointServiceCollectionExtensions.cs

src/Management/src/Endpoint/Middleware/EndpointMiddleware.cs

bart-vmware · 2025-01-17T11:50:27Z

src/Management/src/Endpoint/Actuators/Health/HealthConverter.cs

            writer.WriteStartObject();

-            foreach ((string detailKey, object detailValue) in value.Details)
+            foreach (KeyValuePair<string, object> detail in value.Details)


Please deconstruct into:

foreach ((string componentKey, object componentValue) in value.Details)

HealthConverter has been removed

bart-vmware · 2025-01-17T12:15:44Z

src/Management/test/Endpoint.Test/Actuators/Health/EndpointMiddlewareTest.cs

-        health.Should().ContainKey("details");
-        var details = JsonSerializer.Deserialize<Dictionary<string, object>>(health!["details"].ToString()!);
+        health.Should().ContainKey("components");
+        var details = JsonSerializer.Deserialize<Dictionary<string, object>>(health!["components"].ToString()!);


~~var details~~ var components

bart-vmware · 2025-01-17T13:02:51Z

src/Management/src/Endpoint/Actuators/ThreadDump/EndpointServiceCollectionExtensions.cs

@@ -38,58 +38,22 @@ public static IServiceCollection AddThreadDumpActuator(this IServiceCollection s
    /// The incoming <paramref name="services" /> so that additional calls can be chained.
    /// </returns>
    public static IServiceCollection AddThreadDumpActuator(this IServiceCollection services, bool configureMiddleware)
-    {


The following note should be removed from the documentation at https://github.com/SteeltoeOSS/Documentation/blob/v4/api/v4/management/threaddump.md:

bart-vmware · 2025-01-17T13:03:23Z

src/Management/test/Endpoint.Test/Actuators/ThreadDump/EndpointMiddlewareTest.cs

@@ -21,8 +21,7 @@ public sealed class EndpointMiddlewareTest : BaseTest
    {
        ["management:endpoints:enabled"] = "true",
        ["management:endpoints:dump:enabled"] = "true",
-        ["management:endpoints:actuator:exposure:include:0"] = "threaddump",
-        ["management:endpoints:actuator:exposure:include:1"] = "dump"
+        ["management:endpoints:actuator:exposure:include:0"] = "threaddump"


I find it confusing that we now have an actuator with the ID "threaddump" at /actuator/threaddump, while its configuration key is management:endpoints:dump, especially since we also have management:endpoints:heapdump.

I'd like to propose changing the configuration key to match up (which is a breaking change that we should call out in the docs)

Yeah, I was planning to break things up a little, but this got big enough that it doesn't make sense to do separately so is now included

src/Management/test/Endpoint.Test/Actuators/ThreadDump/ThreadDumpEndpointTest.cs

bart-vmware · 2025-01-17T13:47:46Z

src/Management/test/Endpoint.Test/Actuators/Health/EndpointMiddlewareTest.cs

In the following tests:

HealthActuator_ReturnsOnlyStatus

HealthActuator_ReturnsOnlyStatus_WhenAuthorizedSetButUserIsNot

HealthActuator_ReturnsDetailsWhenConfigured

The line:

health.Should().NotContainKey("details");

should be removed. Because details is a child of components, so it can never occur as a key in the health dictionary.

And I think that comments like:

// {"status":"UP","components":{"diskSpace":{"status":"UP","details":{"total":1003588939776...

should be removed, because they are no longer correct. They add confusion as soon as the comment differs from a changed implementation, which we won't get any warnings/failures about.

Co-authored-by: Bart Koelman <104792814+bart-vmware@users.noreply.github.com>

…e same, single media type)

drop HealthConverter align probe contributor Ids with Spring disk contributor: add path and exists details, drop status Groups: - decouple default group and contributor names - use PostConfigure to set defaults

allow "v2" responses where SBA requires v2 and there is no real difference

sonarqubecloud · 2025-01-25T02:32:49Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

TimHess added Component/Management Issues related to Steeltoe Management (actuators) ReleaseLine/4.x Identified as a feature/fix for the 4.x release line labels Jan 16, 2025

TimHess self-assigned this Jan 16, 2025

bart-vmware requested changes Jan 17, 2025

View reviewed changes

bart-vmware added this to the 4.0.0-m1 milestone Jan 21, 2025

TimHess changed the title ~~Remove actuator content negotiation, only support v3~~ Actuator content negotiation Jan 21, 2025

TimHess force-pushed the remove_negotiation branch from a83dfac to 50fdd11 Compare January 22, 2025 00:56

TimHess added Warn/breaking-change There is a breaking change in the code Warn/api-change The APIs were changed and removed Warn/breaking-change There is a breaking change in the code labels Jan 22, 2025

TimHess and others added 4 commits January 21, 2025 19:07

Actuators now return only application/vnd.spring-boot.actuator.v3+json

b4edc74

Apply suggestions from code review

15110fd

Co-authored-by: Bart Koelman <104792814+bart-vmware@users.noreply.github.com>

Adds minimal (hard-coded) content negotiation (every actuator uses th…

de1f85b

…e same, single media type)

Re-add rudimentary content negotiation

47d8e71

drop HealthConverter align probe contributor Ids with Spring disk contributor: add path and exists details, drop status Groups: - decouple default group and contributor names - use PostConfigure to set defaults

TimHess force-pushed the remove_negotiation branch from 50fdd11 to 47d8e71 Compare January 22, 2025 01:13

TimHess added 2 commits January 24, 2025 19:43

less public api version handling

6cffbc0

allow "v2" responses where SBA requires v2 and there is no real difference

drop duplicated status in health check details

b127eee

TimHess force-pushed the remove_negotiation branch from 80a3801 to b127eee Compare January 25, 2025 01:44

TimHess added 2 commits January 24, 2025 20:03

remove unused usings

f96b93f

removing another unused using

af91998

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Actuator content negotiation #1438

Actuator content negotiation #1438

TimHess commented Jan 16, 2025 •

edited

Loading

bart-vmware left a comment

bart-vmware Jan 17, 2025

TimHess Jan 22, 2025

bart-vmware Jan 17, 2025

bart-vmware Jan 17, 2025

bart-vmware Jan 17, 2025

TimHess Jan 22, 2025

bart-vmware Jan 17, 2025

sonarqubecloud bot commented Jan 25, 2025

Actuator content negotiation #1438

Are you sure you want to change the base?

Actuator content negotiation #1438

Conversation

TimHess commented Jan 16, 2025 • edited Loading

Description

Quality checklist

bart-vmware left a comment

Choose a reason for hiding this comment

bart-vmware Jan 17, 2025

Choose a reason for hiding this comment

TimHess Jan 22, 2025

Choose a reason for hiding this comment

bart-vmware Jan 17, 2025

Choose a reason for hiding this comment

bart-vmware Jan 17, 2025

Choose a reason for hiding this comment

bart-vmware Jan 17, 2025

Choose a reason for hiding this comment

TimHess Jan 22, 2025

Choose a reason for hiding this comment

bart-vmware Jan 17, 2025

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 25, 2025

Quality Gate passed

TimHess commented Jan 16, 2025 •

edited

Loading