Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Duplicate timestamp in _trial_temp/test.log log formatting #15618

Closed
1 task
MadLittleMods opened this issue May 18, 2023 · 2 comments · Fixed by #15636
Closed
1 task

Duplicate timestamp in _trial_temp/test.log log formatting #15618

MadLittleMods opened this issue May 18, 2023 · 2 comments · Fixed by #15636
Assignees
Labels
A-Logging Synapse's logs (structured or otherwise). Not metrics. A-Testing Issues related to testing in complement, synapse, etc T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@MadLittleMods
Copy link
Contributor

MadLittleMods commented May 18, 2023

Currently, the log lines in _trial_temp/test.log seem way too verbose and duplicate the timestamp for some reason. It's the exact same time in both places but one with timezone offset and one with millisecond precision.

_trial_temp/test.log

2023-05-17 22:51:36-0500 [-] 2023-05-17 22:51:36,889 - synapse.server - 338 - INFO - sentinel - Finished setting up.

This is in contrast to my own homeserver.log which uses the same logging format as what the tests seem to use but only has the asctime as expected,

log_format = (
"%(asctime)s - %(name)s - %(lineno)d - "
"%(levelname)s - %(request)s - %(message)s"
)

If you remove %(asctime)s - from the test logging setup, the logs in _trial_temp/test.log still end up with a timestamp 🤔:

2023-05-18 00:41:35-0500 [-] synapse.server - 338 - INFO - sentinel - Finished setting up.

Definition of done

  • Remove duplicate timestamps from _trial_temp/test.log log formatting
@MadLittleMods MadLittleMods added A-Logging Synapse's logs (structured or otherwise). Not metrics. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. A-Testing Issues related to testing in complement, synapse, etc labels May 18, 2023
@reivilibre
Copy link
Contributor

May be worth checking whether it's part of Trial itself

@MadLittleMods MadLittleMods self-assigned this May 19, 2023
@MadLittleMods
Copy link
Contributor Author

It looks like this is from the Twisted logger: twisted/twisted -> src/twisted/logger/_format.py#L362-L374

It doesn't seem that Twisted really supports changing the format unless you override the emit method:

The easiest thing to do is probably just remove %(asctime)s - from our format

MadLittleMods added a commit that referenced this issue May 22, 2023
…15636)

Fix #15618

### Before

```
2023-05-17 22:51:36-0500 [-] 2023-05-17 22:51:36,889 - synapse.server - 338 - INFO - sentinel - Finished setting up.
```

### After

```
2023-05-19 18:16:20-0500 [-] synapse.server - 338 - INFO - sentinel - Finished setting up.
```


### Dev notes

The `Twisted.Logger` controls the `2023-05-19 18:16:20-0500 [-]` prefix, see : [`twisted/twisted` -> `src/twisted/logger/_format.py#L362-L374`](https://github.com/twisted/twisted/blob/34b161e66bc7c9f9efbb95e82c770a863933e498/src/twisted/logger/_format.py#L362-L374)

And we delegate our logs to the Twisted Logger for the tests which puts it in `_trial_temp/test.log`
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Logging Synapse's logs (structured or otherwise). Not metrics. A-Testing Issues related to testing in complement, synapse, etc T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants