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

refactor(runtime,codegen): avoid double timestamps problem #1210

Conversation

iulianbarbu
Copy link
Contributor

@iulianbarbu iulianbarbu commented Sep 7, 2023

Description of change

Rebased on top of #1209.

  • Runtime resource tracker emit logs prepended by a timestamp we add explicitly. This was removed.
  • Our default tracing fmt layer formats events by prepending a timestamp, but we also emit the timestamp for each line from the runtime stdout collecting logic. I changed the default tracing layer to format events without a timestamp.

Note:

  • users custom tracing is not touched and if they emit timestamps they will pop up in the deployment logs. I think this should be expected behavior and leave the users the choice to format their logs as wanted.

How has this been tested? (if applicable)

Tested locally:

  • with the default tracing
  • without the default tracing

Copy link
Member

@chesedo chesedo left a comment

Choose a reason for hiding this comment

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

Seems good to go. Just need to remove the changes from #1209

@iulianbarbu iulianbarbu force-pushed the feature/eng-1216-cleanup-runtime-logs-from-double-timestamps branch from 245d121 to b87463a Compare September 7, 2023 16:37
Copy link
Member

@chesedo chesedo left a comment

Choose a reason for hiding this comment

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

LGTM

@iulianbarbu iulianbarbu merged commit a7d0ee0 into shuttle-hq:feat/shuttle-logger-service Sep 8, 2023
18 of 34 checks passed
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.

None yet

4 participants