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

Pass expected name for enabling structured logging in providers #558

Merged

Conversation

connorsmith256
Copy link
Contributor

Feature or Problem

Currently, the host passes enable_structured_logging as a field in the HostData given to providers via stdin when they start. However, the smithy definition of HostData expects the field to be serialized as structured_logging.

In effect, there is no way to enable structured logging in providers today, since we've misspelled the field.

Related Issues

This doesn't fix wasmCloud/weld#140, but it is related

Release Information

Next release

Consumer Impact

The impact should be positive, since we're enabling structured logs for providers, unifying the log format between the host and providers. However, on the off chance that users are relying on this buggy behavior today, this could technically be considered a breaking change(?)

Testing

Unit Test(s)

N/A

Acceptance or Integration

N/A

Manual Verification

  • started a host with WASMCLOUD_STRUCTURED_LOGGING_ENABLED=true WASMCLOUD_STRUCTURED_LOG_LEVEL=debug make run
  • started the HTTP server provider, echo actor, and linked them
  • made a request and confirmed the provider emitted structured log messages like {"timestamp":"2023-03-07T16:44:24.744883Z","level":"INFO","fields":{"message":"finished processing with success","status":200},"target":"warp::filters::trace"}

@connorsmith256 connorsmith256 changed the title pass expected name for enabling structured logging in Rust providers pass expected name for enabling structured logging Mar 7, 2023
@connorsmith256 connorsmith256 changed the title pass expected name for enabling structured logging Pass expected name for enabling structured logging Mar 7, 2023
@connorsmith256 connorsmith256 changed the title Pass expected name for enabling structured logging Pass expected name for enabling structured logging in providers Mar 7, 2023
brooksmtownsend
brooksmtownsend previously approved these changes Mar 7, 2023
thomastaylor312
thomastaylor312 previously approved these changes Mar 7, 2023
Copy link
Contributor

@thomastaylor312 thomastaylor312 left a comment

Choose a reason for hiding this comment

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

This is a breaking change, although small. So you were thinking right

@connorsmith256
Copy link
Contributor Author

Discussed offline, but determined this is not a breaking change, so I'm not going to bump the version in this PR

@connorsmith256 connorsmith256 merged commit 8881fcb into wasmCloud:main Mar 10, 2023
@connorsmith256 connorsmith256 deleted the fix/structured-log-host-data branch March 10, 2023 18:36
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.

[QUESTION] Should log level be included in HostData?
4 participants