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

Event log: enable custom timestamp (via message prefix) #10

Closed
wants to merge 4 commits into from

Conversation

metachris
Copy link
Contributor

@metachris metachris commented Nov 21, 2024

📝 Summary

Allow custom timestamps in the event log.

If a message is prefixed with a timestamp (in sec or ms), then that part is used for ReceivedAt and the message is the remainder of the original message:

# Start the server
$ go run cmd/system-api/main.go

# Add regular event
$ echo "hello world" > pipe.fifo

# Add event with custom timestamp
$ echo "1634966400 this is a test" > pipe.fifo

# Query events
$ curl localhost:3535/logs
2024-11-21T19:48:04Z     hello world
2021-10-23T05:20:00Z     1634966400 this is a test      <--- custom timestamp on this entry

Motivation

Some services might log into an (async) named pipe already before system-api has started. Without custom timestamps, those log entries would all have the timestamp when system-api started, instead of when the log message was written.

Downsides

Allows services to mess with internals through the log message, and spoof the timestamp of it's own event arbitrarily.

Discussion

Not yet sure what's the best path forward -- giving services this freedom, or simply not having 'proper' timestamps for logs received before system-api start.


✅ I have run these commands

  • make lint
  • make test
  • go mod tidy

@metachris metachris changed the title Parse timestamp Event log: enable custom timestamp (via message prefix) Nov 21, 2024
@bakhtin
Copy link
Contributor

bakhtin commented Nov 22, 2024

Allows services to mess with internals through the log message
IMO, a reasonable trade-off in the current setup.

Do you think we can add a boolean config flag like timestamp_override to toggle timestamps override from the messages?

README.md Show resolved Hide resolved
@metachris
Copy link
Contributor Author

Do you think we can add a boolean config flag like timestamp_override to toggle timestamps override from the messages?

Sure we can do that, and that's nice. But it doesn't really change the issue because we'd usually ship it with that functionality enabled.

@metachris
Copy link
Contributor Author

closing, because magic and side-effects are not desirable here

@metachris metachris closed this Nov 25, 2024
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.

3 participants