fix(telemetry): Empty context persisted when remaining beans are negative after run finish #10635
Conversation
| @@ -356,7 +356,11 @@ export const makeContextualSlogProcessor = ( | |||
| // eslint-disable-next-line no-restricted-syntax | |||
| case SLOG_TYPES.COSMIC_SWINGSET.RUN.FINISH: { | |||
| assert(!!triggerContext); | |||
| persistContext(finalBody.remainingBeans ? {} : triggerContext); | |||
| persistContext( | |||
| finalBody.remainingBeans && finalBody.remainingBeans > 0 | |||
There was a problem hiding this comment.
I sometimes dislike what types force us to do. The finalBody.remainingBeans && part is totally unnecessary at runtime.
|
|
||
| // eslint-disable-next-line prefer-template | ||
| stream.write(serializeSlogObj(contextualizedSlog) + '\n').catch(() => {}); | ||
| stream.write(serializeSlogObj({ time, ...rest }) + '\n').catch(() => {}); |
There was a problem hiding this comment.
I would to keep the order or these fields sane when a human (me) is reading the file in a terminal or editor with no line wrap.
| stream.write(serializeSlogObj({ time, ...rest }) + '\n').catch(() => {}); | |
| stream.write(serializeSlogObj({ ...rest, time }) + '\n').catch(() => {}); |
I kinda wish the process.uptime moved to the end of the attributes too.
Similarly let's move the body before attributes in the processor. Btw why not rename the time field there instead of here? We already have to extract and process it in the otel sender, so there's no extra work there, and we'd avoid extra work here.
There was a problem hiding this comment.
I have moved process.uptime to the end, but GCP still order the keys
Same with attributes and body
There was a problem hiding this comment.
Yup but at least the file output is maintaining the order.
Deploying agoric-sdk with
|
| Latest commit: |
5eff6ba
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5f867ff6.agoric-sdk.pages.dev |
| Branch Preview URL: | https://usman-misc-slogger-fixes.agoric-sdk.pages.dev |
|
This pull request has been removed from the queue for the following reason: Pull request #10635 has been dequeued. The pull request has been merged manually. The pull request has been merged manually at ad4e83e. You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it. If you want to requeue this pull request, you need to post a comment with the text: |
…tive after run finish (#10635)
### Description Cherry-picks the following commits from master: - #10551 (9e19321) - #10635 (ad4e83e) - #10615 (e596a01 ) - #10634 (a1856f3) Since we plan to verify this rc on devnet rather than emerynet, there is no apparent need for a new upgrade name. Not aware of any deployments on devnet before so can reuse the previous upgrade name. However, skipping emerynet is dependent on comms with the validators so I have added a new upgrade name `agoric-ugprade-18-emerynet-rc3` just in case. Only added to bypass the need for a new rc if for some reason emerynet validation is needed anyways - best case scenario is that it remains unused. commits added using git cherry-pick
## Description Created as per MAINTAINERS.md ## Changes - @agoric/cosmos@0.35.0-u18.3 - agoric@0.22.0-u18.3 - @agoric/benchmark@0.1.1-u18.3 - @agoric/boot@0.2.0-u18.3 - @agoric/builders@0.2.0-u18.3 - @agoric/casting@0.4.3-u18.3 - @agoric/cosmic-proto@0.5.0-u18.3 - @agoric/cosmic-swingset@0.42.0-u18.3 - @agoric/create-dapp@0.1.1-u18.3 - @agoric/deploy-script-support@0.10.4-u18.3 - fast-usdc@0.1.1-u18.3 - @agoric/inter-protocol@0.17.0-u18.3 - @agoric/orchestration@0.2.0-u18.3 - @agoric/pegasus@0.8.0-u18.3 - @agoric/smart-wallet@0.5.4-u18.3 - @agoric/solo@0.11.0-u18.3 - @agoric/swingset-runner@0.22.3-u18.3 - @agoric/telemetry@0.6.3-u18.2 - @agoric/vats@0.16.0-u18.3 - @agoric/wallet-backend@0.15.0-u18.3 ## Packages that have NEWS.md updates ```diff --- a/packages/telemetry/CHANGELOG.md +++ b/packages/telemetry/CHANGELOG.md @@ -3,6 +3,16 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +### [0.6.3-u18.2](https://github.com/Agoric/agoric-sdk/compare/@agoric/telemetry@0.6.3-u18.1...@agoric/telemetry@0.6.3-u18.2) (2024-12-09) + + +### Bug Fixes + +* **telemetry:** Empty context persisted when remaining beans are negative after run finish ([#10635](#10635)) ([3988aa0](3988aa0)) +* **telemetry:** event name typo ([070b154](070b154)) + + + ### [0.6.3-u18.1](https://github.com/Agoric/agoric-sdk/compare/@agoric/telemetry@0.6.3-u18.0...@agoric/telemetry@0.6.3-u18.1) (2024-11-19) ```
refs: #10300
Description
Due to a faulty condition, empty context was persisted in case remaining beans were negative after a run finish
This PR also renames
timestamptotimein thecontext-aware-slog-file.jsslogger so that the timestamp is automatically picked up by GCP Stack driver as the time of the logSecurity Considerations
None
Scaling Considerations
None
Documentation Considerations
None
Testing Considerations
None
Upgrade Considerations
None