spire-agent: implement logger service by sorindumitru · Pull Request #7017 · spiffe/spire

sorindumitru · 2026-06-03T06:54:26Z

Pull Request check list

Commit conforms to CONTRIBUTING.md?
Proper tests/regressions included?
Documentation updated?

Affected functionality

Description of change

Which issue this PR fixes

Copilot

Pull request overview

This PR adds an admin “logger” service to the SPIRE Agent and wires it into the agent’s admin API endpoints, along with corresponding spire-agent logger {get,set,reset} CLI commands and integration/unit test coverage.

Changes:

Add pkg/agent/api/logger/v1 gRPC service to query and mutate the agent’s root log level (including reset to launch level).
Add spire-agent logger get|set|reset CLI commands (with shared client/flag adapter utilities).
Add unit tests for logger level mappings and service behavior, plus an integration test suite covering end-to-end behavior.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
test/integration/suites/agent-cli/08-logger	Adds an integration suite validating log level get/set/reset and expected logging behavior.
pkg/agent/config.go	Changes agent config logger type to support GetLevel/SetLevel.
pkg/agent/api/logger/v1/service.go	Implements the agent admin Logger service (get/set/reset).
pkg/agent/api/logger/v1/service_test.go	Adds unit tests for the logger service RPC behavior and logging expectations.
pkg/agent/api/logger/v1/levels.go	Adds mappings between logrus levels and API enum levels.
pkg/agent/api/logger/v1/levels_test.go	Adds unit tests validating the level mapping tables.
pkg/agent/api/endpoints.go	Registers the new logger API endpoint on the agent admin API server.
pkg/agent/api/config.go	Adds `RootLog` to supply a mutable root logger to the logger service.
pkg/agent/agent.go	Wires the agent’s root logger into admin API config.
cmd/spire-agent/util/util.go	Adds shared CLI adapter/client utilities used by logger commands.
cmd/spire-agent/util/util_windows.go	Windows implementation for admin API address flag + named pipe dialer.
cmd/spire-agent/util/util_posix.go	POSIX implementation for admin API address flag + unix socket dialer.
cmd/spire-agent/cli/logger/get.go	Adds `spire-agent logger get` command.
cmd/spire-agent/cli/logger/get_test.go	Tests `logger get` output and error handling.
cmd/spire-agent/cli/logger/set.go	Adds `spire-agent logger set` command.
cmd/spire-agent/cli/logger/set_test.go	Tests `logger set` behavior and error cases.
cmd/spire-agent/cli/logger/reset.go	Adds `spire-agent logger reset` command.
cmd/spire-agent/cli/logger/reset_test.go	Tests `logger reset` behavior and error cases.
cmd/spire-agent/cli/logger/printers.go	Adds custom pretty printer for logger responses.
cmd/spire-agent/cli/logger/printers_test.go	Tests pretty printer behavior and error propagation.
cmd/spire-agent/cli/logger/mocks_test.go	Provides CLI test harness and mock Logger gRPC service.
cmd/spire-agent/cli/logger/logger_windows_test.go	Windows-specific CLI flag usage expectations.
cmd/spire-agent/cli/logger/logger_posix_test.go	POSIX-specific CLI flag usage expectations.
cmd/spire-agent/cli/cli.go	Registers logger subcommands in the agent CLI command map.

+func New(c Config) *Service {
+	launchLogLevel := c.Log.GetLevel()
+	c.Log.WithFields(logrus.Fields{
+		telemetry.LaunchLogLevel: launchLogLevel,


+
+func (s *Service) ResetLogLevel(ctx context.Context, _ *loggerv1.ResetLogLevelRequest) (*apitype.Logger, error) {
+	log := rpccontext.Logger(ctx)
+	log.WithField(telemetry.LaunchLogLevel, s.launchLevel).Info("ResetLogLevel Called")


+func (s *Service) SetLogLevel(ctx context.Context, req *loggerv1.SetLogLevelRequest) (*apitype.Logger, error) {
+	rpccontext.AddRPCAuditFields(ctx, logrus.Fields{telemetry.NewLogLevel: req.NewLevel})
+	log := rpccontext.Logger(ctx)
+
+	if req.NewLevel == apitype.LogLevel_UNSPECIFIED {
+		return nil, api.MakeErr(log, codes.InvalidArgument, "newLevel value cannot be LogLevel_UNSPECIFIED", nil)
+	}
+
+	newLogLevel, ok := LogrusLevel[req.NewLevel]
+	if !ok {
+		return nil, api.MakeErr(log, codes.InvalidArgument, "unsupported log level", nil)
+	}
+
+	log.WithFields(logrus.Fields{
+		telemetry.NewLogLevel: newLogLevel.String(),
+	}).Info("SetLogLevel Called")
+	s.log.SetLevel(newLogLevel)


c4rlo · 2026-06-06T10:29:39Z

+type errorWriter struct {
+	ReturnError error
+	buf         strings.Builder
+}


I see you added this here because this file uses that type. But you've still got another copy of it in cmd/spire-server/cli/logger/mocks_test.go and another one in cmd/spire-agent/cli/logger/mocks_test.go, both of which are now unused.

Thanks for catching this

c4rlo · 2026-06-06T10:37:49Z

+
+	if req.NewLevel == apitype.LogLevel_UNSPECIFIED {
+		return nil, api.MakeErr(log, codes.InvalidArgument, "newLevel value cannot be LogLevel_UNSPECIFIED", nil)
+	}


Seems like this would be caught by the LogrusLevel lookup below so can remove this check.

Yeah, maybe. This mostly follows the server side where I think it wants to give a better error message for the case where you forgot to put in a value in the request, as opposed to putting in some invalid value.

I agree the next check catches this, but I'll leave this in to be consistent with the server side and to get marginally better error messages.

c4rlo · 2026-06-06T10:39:53Z

There are a lot of very similar test cases in here – not sure they're all worth their keep...

Yes, again this mostly follows the server side of things which has a similar array of tests. I agree it's a bit much for the functionality here, but it follows what we do for the server.

rausingh-rh · 2026-06-11T05:14:46Z

This PR will close issue #4986

rausingh-rh · 2026-06-11T10:04:41Z

+	"github.com/spiffe/spire/pkg/server/api"
+	"github.com/spiffe/spire/pkg/server/api/rpccontext"


importing pkg/server/api/rpccontext and pkg/server/api creates a cross-boundary dependency where the agent package depends on server internals. The agent has its own pkg/agent/api/rpccontext package can we use that- so that we are consistent with how it's done in DelegatedIdentity?

This ended up being a bigger change because it was a pre-existing issue, but it makes sense to fix it. Kept it in a separate commit

rausingh-rh · 2026-06-11T10:14:20Z

There's no AgentLoggerServiceName in pkg/common/api/middleware/names.go and no Logger switch case in pkg/agent/endpoints/metrics.go. Due to this, every Logger API call will hit the default branch switch case block and emit an unrecognized service for connection metrics error log. Similar issue was fixed in #6878 for the Debug API. The existing TestAllAgentServicesHandledByConnectionMetrics in test should catch this. worth running it with the Logger service registered to verify.

rausingh-rh · 2026-06-11T10:37:15Z

+	}).Info("SetLogLevel Called")
+	s.log.SetLevel(newLogLevel)
+
+	rpccontext.AuditRPC(ctx)


AuditRPC calls in this file will silently no-op on the agent since the agent middleware doesn't set up audit tracking on the context

rausingh-rh · 2026-06-11T10:51:31Z

+
+| Command       | Action                             | Default                          |
+|:--------------|:-----------------------------------|:---------------------------------|
+| `-socketPath` | Path to the SPIRE Agent API socket | /tmp/spire-agent/public/api.sock |


CLI defaults to /tmp/spire-agent/private/admin.sock for -socketPath. Also the description should say admin API socket instead of API socket since the Logger service is only registered on the admin server, not the workload API server.

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

there's no auditing support in the agent Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

MarcosDY · 2026-06-20T15:37:51Z

 	HealthServiceName                  = "grpc.health.v1.Health"
 	HealthServiceShortName             = "Health"
-	LoggerServiceName                  = "logger.v1.Logger"
+	LoggerServiceName                  = "spire.api.agent.logger.v1.Logger"


I think this LoggerServiceName change (logger.v1.Logger to spire.api.agent.logger.v1.Logger) might regress the server side, it is shared for both.

spire.api.server.logger.v1.Logger from Logger to logger.v1.Logger while spire.api.agent.logger.v1.Logger from spire.api.agent.Logger to Logger,.

So the agent resolves cleanly now but the server logger's metric key regresses from logger.<method> to logger.v1.logger.<method>,
Looks like there is no test for logger naming in pkg/common/api/middleware/*_test.go to catch this.

Can you add a full name for agent and keep the old one at it was?
and maybe add a test to catch it?
something like:

for _, tt := range []struct { name string fullMethod string }{ {name: "server", fullMethod: "/spire.api.server.logger.v1.Logger/SetLogLevel"}, {name: "agent", fullMethod: "/spire.api.agent.logger.v1.Logger/SetLogLevel"}, } { t.Run(tt.name, func(t *testing.T) { names := makeNames(tt.fullMethod) assert.Equal(t, "Logger", names.Service) assert.Equal(t, []string{"logger", "set_log_level"}, names.MetricKey) }) }

Fixed this now. It's good to have the test, it shows the logger service is a bit different, it doesn't include the v1 in the service name. Same for the delegated API one.

We can see about fixing them in 1.16.0, since it's a breaking change.

MarcosDY · 2026-06-20T15:39:14Z

+		// input
+		args []string
+		// expected items
+		expectedSetValue types.LogLevel


Looks like expected is never used?
can you add require.Equal(t, tt.expectedSetValue, *test.server.receivedSetValue)?

Same problem in the server code, fixed it there too

MarcosDY · 2026-06-20T15:42:27Z

+
+| Command       | Action                             | Default                             |
+|:--------------|:-----------------------------------|:------------------------------------|
+| `-socketPath` | Path to the SPIRE Agent API socket | /tmp/spire-agent/private/admin.sock |


NIT: can you update SPIRE Agent API socket to SPIRE Agent admin API socket
Just to avoid confusion with another apis

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

Copilot AI review requested due to automatic review settings June 3, 2026 06:54

sorindumitru requested review from MarcosDY, amartinezfayo, evan2645 and rturner3 as code owners June 3, 2026 06:54

Copilot started reviewing on behalf of sorindumitru June 3, 2026 06:54 View session

Copilot AI reviewed Jun 3, 2026

View reviewed changes

sorindumitru force-pushed the agent-logger-api branch 4 times, most recently from 60922df to e883507 Compare June 3, 2026 12:22

sorindumitru marked this pull request as draft June 3, 2026 13:10

sorindumitru force-pushed the agent-logger-api branch 2 times, most recently from 09ee547 to fbebf1c Compare June 4, 2026 17:30

sorindumitru marked this pull request as ready for review June 4, 2026 17:44

sorindumitru force-pushed the agent-logger-api branch from 3a55c9d to 5cb1db4 Compare June 4, 2026 19:15

c4rlo reviewed Jun 6, 2026

View reviewed changes

sorindumitru force-pushed the agent-logger-api branch from 4a89d06 to 69e71e6 Compare June 7, 2026 09:00

MarcosDY self-assigned this Jun 9, 2026

sorindumitru mentioned this pull request Jun 10, 2026

Support runtime log level adjustment on spire-agent #7057

Closed

rausingh-rh reviewed Jun 11, 2026

View reviewed changes

sorindumitru force-pushed the agent-logger-api branch 2 times, most recently from b76642f to f4def57 Compare June 14, 2026 07:53

sorindumitru added 3 commits June 14, 2026 09:18

spire-agent: implement logger service

9a91f79

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

Copilot review comments

d0eaebd

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

review comments

1b251d6

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

sorindumitru added 7 commits June 14, 2026 09:18

review comments

17f5b7f

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

Remove auditing

ca53855

there's no auditing support in the agent Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

Use agent rpccontext

5ae8f6c

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

Move status helpers to common/api

0e224db

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

Use status helpers from common/api

b01ca9a

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

Make sure logger metrics are emitted

c2eec37

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

fix linter errors

a426dc6

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

sorindumitru force-pushed the agent-logger-api branch from f4def57 to a426dc6 Compare June 14, 2026 08:18

MarcosDY reviewed Jun 20, 2026

View reviewed changes

github-actions Bot assigned sorindumitru and unassigned MarcosDY Jun 20, 2026

Review comments

a570c7f

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

sorindumitru assigned MarcosDY and unassigned sorindumitru Jun 21, 2026

sorindumitru requested a review from MarcosDY June 21, 2026 06:05

Add RPC metric name test

a99635b

Signed-off-by: Sorin Dumitru <sorin@returnze.ro>

sorindumitru force-pushed the agent-logger-api branch from 0997740 to a99635b Compare June 21, 2026 06:38

		"github.com/spiffe/spire/pkg/server/api"
		"github.com/spiffe/spire/pkg/server/api/rpccontext"

Conversation

sorindumitru commented Jun 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rausingh-rh commented Jun 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rausingh-rh commented Jun 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants