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

Support retuning of log level on spire-{server,agent} #4785

Closed
edwbuck opened this issue Jan 9, 2024 · 7 comments · Fixed by #4880
Closed

Support retuning of log level on spire-{server,agent} #4785

edwbuck opened this issue Jan 9, 2024 · 7 comments · Fixed by #4880
Labels
priority/backlog Issue is approved and in the backlog unscoped The issue needs more design or understanding in order for the work to progress

Comments

@edwbuck
Copy link
Contributor

edwbuck commented Jan 9, 2024

Post-launch, it would be beneficial to adjust the SPIRE log level. The server should read the initial log level from the config file path / command line options but then accept a cli command to adjust the SPIRE Server instance previously launched with spire-server run ...

One approach would be to use the SPIRE CLI. The proposed new command would use the format of "EXECUTABLE adjustlog TARGET <LOG_LEVEL>|DEFAULT" The TARGET parameter will help for future efforts should we ever make the plugin logging independent of the the server/agent logging.

A few examples of the proposed command:

  • spire adjustlog server INFO
  • spire adjustlog server WARN
  • spire adjustlog server DEFAULT
  • spire adjustlog DataStore:sql DEBUG
  • spire adjustlog UpstreamAuthority:disk ERROR
  • spire adjustlog agent INFO

Where the values are a combination of the log levels supported by server.log_level and the additional word DEFAULT which resets the log level back to its launch setting.

To query the runtime log level an optional command might also be added

  • spire getlog server

Which might report one of <DEBUG|INFO|WARN|ERROR> based on the current log level, which might be overriding the configuration file.

@evan2645 evan2645 added the triage/in-progress Issue triage is in progress label Jan 9, 2024
@azdagron
Copy link
Member

I think if we take this, we'd probably want to constrain this to process-wide level changes only.

@edwbuck
Copy link
Contributor Author

edwbuck commented Jan 16, 2024

Outstanding questions:

Do we create an administrative networked API for this?
Do we constrain it to only the UNIX domain socket?

@azdagron
Copy link
Member

Switching log levels at runtime is a reasonable feature request. We need to scope out a few details:

  1. Granularity: I think we'd prefer process-wide only at this point
  2. Mechanism for adjustment: New API? Hosted on UDS only or available over TCP?
  3. Access control to that mechanism

@azdagron azdagron added priority/backlog Issue is approved and in the backlog unscoped The issue needs more design or understanding in order for the work to progress and removed triage/in-progress Issue triage is in progress labels Jan 16, 2024
@edwbuck
Copy link
Contributor Author

edwbuck commented Jan 23, 2024

  • Do we create an administrative networked API for this?

No. Network access for this API is not required.

  • Do we constrain it to only the UNIX domain socket? Hosted on UDS only or available over TCP?

The API should be constrained to the Server UDS and the Agent "Admin UDS" socket.

  • Mechanism for adjustment: New API?

A new UDP API is required.

  • Access control to that mechanism?

Access to the cli will assume an Administrator, much in the same way that we currently assume Administrator

  • Granularity: I think we'd prefer process-wide only at this point

Named logging support should be included in this effort. This is due the requirement to tune plugin log entries separately specifically to debug custom in-house written plugins independently of the rest of the system.

The API would change if we don't support the readjustment of sub-loggers. It would likely be a better solution if we could handle different logging of different Named loggers upfront, as odds are we will have to handle these issues anyway with the fixes prescribed to the HCLogAdapter, which would permit plugin logging access.

@edwbuck
Copy link
Contributor Author

edwbuck commented Jan 23, 2024

The current design is to put a lightweight struct around the logrus.Logger, holding the logger name and the logger level (a uInt).

These entries would be stored in a log.Registry which would contain a map["logger name"]"log wrapper"

The log wrapper would implement the logrus.FieldLogger interface for compatibility with the current system, and would ideally also implement a hclog.Logger interface or at least support it fully (the current logrus.FieldLogger is notably missing support for hclog's SetLevel and Named loggers).

@edwbuck
Copy link
Contributor Author

edwbuck commented Jan 30, 2024

Requesting that this item no longer be unscoped, as the scope of work has been defined.

@edwbuck
Copy link
Contributor Author

edwbuck commented Jan 30, 2024

API updates for the UDP admin sockets are proposed here spiffe/spire-api-sdk#54

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/backlog Issue is approved and in the backlog unscoped The issue needs more design or understanding in order for the work to progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants