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

Reformat timestamps in factorio log #181

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

trigrab
Copy link

@trigrab trigrab commented Aug 25, 2020

While using your very useful server manager, it always bothered me that you can't see the real date/time in the log file.

Today I realized, that this has nothing to do with your software but with the format factorio stores the logs in. So I made this PR to reformat the log.

Maybe we could also implement a switch to toggle this feature. But as this would be part of the frontend and I do not know a lot about react, I will leave it as is.

This is my first code in go, so I am very open for suggestions depending code quality and style. Of course any other suggestions are welcome as well.

Hope to see this in upstream soon.

@knoxfighter
Copy link
Member

knoxfighter commented Aug 25, 2020

If im understanding that correctly:
this PR changes 0.000 2020-08-19 14:40:02; Factorio 1.0.0 (build 54889, linux64, headless) to 2020-08-19 14:40:02; Factorio 1.0.0 (build 54889, linux64, headless), correct?

In my logs i find this lineformat exactly once, so im not sure, if this is really something we need.
All other lines are without timestamp like 7808.136 Info AppManagerStates.cpp:1837: Saving finished or only with timestamp and without time since factorio start like 2020-08-19 16:57:07 [CHAT] asdff45: [gps=8,-45]

If you have this often, just write so and change my mind about needing this.

Also a . in a regex means any character, so ^\d+.\d+ matches two numbers, separated by any other character (like 111a111). To match . directly escape it with a backspace, so your final regex is ^\d+.\d+.
Also using Replace instead of your complex code, would make it a lot smaller and easer to read. https://golang.org/pkg/regexp/#Regexp.ReplaceAll

Please use develop as base branch and merge it into your PR branch.

@trigrab
Copy link
Author

trigrab commented Aug 25, 2020

Thanks for your feedback. I will look into it asap and improve my code.

As for my deployment of the server manager, I get only lines like this:

 829.479 Info ServerMultiplayerManager.cpp:769: updateTick(1078173) changing state from(InGame) to(InGameSavingMap)
 829.529 Info ServerMultiplayerManager.cpp:917: updateTick(1078173) received stateChanged peerID(2) oldState(Ready) newState(ConnectedWaitingForMap)
 829.863 Info ServerMultiplayerManager.cpp:984: UpdateTick(1078173) Serving map(/opt/factorio/temp/mp-save-1.zip) for peer(2) size(6152493) crc(16603608)
 829.863 Info ServerMultiplayerManager.cpp:769: updateTick(1078173) changing state from(InGameSavingMap) to(InGame)
 829.949 Info ServerMultiplayerManager.cpp:917: updateTick(1078178) received stateChanged peerID(2) oldState(ConnectedWaitingForMap) newState(ConnectedDownloadingMap)
 831.346 Info ServerMultiplayerManager.cpp:917: updateTick(1078262) received stateChanged peerID(2) oldState(ConnectedDownloadingMap) newState(ConnectedLoadingMap)
 832.072 Info ServerMultiplayerManager.cpp:917: updateTick(1078306) received stateChanged peerID(2) oldState(ConnectedLoadingMap) newState(TryingToCatchUp)
 832.279 Info ServerMultiplayerManager.cpp:917: updateTick(1078318) received stateChanged peerID(2) oldState(TryingToCatchUp) newState(WaitingForCommandToStartSendingTickClosures)
 832.280 Info GameActionHandler.cpp:4725: UpdateTick (1078318) processed PlayerJoinGame peerID(2) playerIndex(1) mode(connect) 
 832.349 Info ServerMultiplayerManager.cpp:917: updateTick(1078322) received stateChanged peerID(2) oldState(WaitingForCommandToStartSendingTickClosures) newState(InGame)
 860.314 Info AppManager.cpp:278: Saving to _autosave5 (non-blocking).
 860.318 Info AsyncScenarioSaver.cpp:144: Saving process PID: 62
 860.933 Info ChildProcessAgent.cpp:60: Child 62 exited with return value 0
 860.934 Info AppManager.cpp:279: Saving finished

Which look like this with my code (line numbers in reverse because I pasted the above from the actual log file):

2020-08-25 20:57:49: Info AppManager.cpp:279: Saving finished
2020-08-25 20:57:49: Info ChildProcessAgent.cpp:60: Child 65 exited with return value 0
2020-08-25 20:57:49: Info AsyncScenarioSaver.cpp:144: Saving process PID: 65
2020-08-25 20:57:49: Info AppManager.cpp:278: Saving to _autosave1 (non-blocking).
2020-08-25 20:47:49: Info AppManager.cpp:279: Saving finished
2020-08-25 20:47:49: Info ChildProcessAgent.cpp:60: Child 62 exited with return value 0
2020-08-25 20:47:49: Info AsyncScenarioSaver.cpp:144: Saving process PID: 62
2020-08-25 20:47:49: Info AppManager.cpp:278: Saving to _autosave5 (non-blocking).
2020-08-25 20:47:21: Info ServerMultiplayerManager.cpp:917: updateTick(1078322) received stateChanged peerID(2)  ldState(WaitingForCommandToStartSendingTickClosures) newState(InGame)
2020-08-25 20:47:21: Info GameActionHandler.cpp:4725: UpdateTick (1078318) processed PlayerJoinGame peerID(2) playerIndex(1) mode(connect)
2020-08-25 20:47:21: Info ServerMultiplayerManager.cpp:917: updateTick(1078318) received stateChanged peerID(2) oldState(TryingToCatchUp) newState(WaitingForCommandToStartSendingTickClosures)
2020-08-25 20:47:21: Info ServerMultiplayerManager.cpp:917: updateTick(1078306) received stateChanged peerID(2) oldState(ConnectedLoadingMap) newState(TryingToCatchUp)
2020-08-25 20:47:20: Info ServerMultiplayerManager.cpp:917: updateTick(1078262) received stateChanged peerID(2) oldState(ConnectedDownloadingMap) newState(ConnectedLoadingMap)
2020-08-25 20:47:18: Info ServerMultiplayerManager.cpp:917: updateTick(1078178) received stateChanged peerID(2) oldState(ConnectedWaitingForMap) newState(ConnectedDownloadingMap)
2020-08-25 20:47:18: Info ServerMultiplayerManager.cpp:769: updateTick(1078173) changing state from(InGameSavingMap) to(InGame)
2020-08-25 20:47:18: Info ServerMultiplayerManager.cpp:984: UpdateTick(1078173) Serving map(/opt/factorio/temp/mp save-1.zip) for peer(2) size(6152493) crc(16603608)
2020-08-25 20:47:18: Info ServerMultiplayerManager.cpp:917: updateTick(1078173) received stateChanged peerID(2) oldState(Ready) newState(ConnectedWaitingForMap)
2020-08-25 20:47:18: Info ServerMultiplayerManager.cpp:769: updateTick(1078173) changing state from(InGame) to(InGameSavingMap)
2020-08-25 20:47:18: Info ServerSynchronizer.cpp:599: nextHeartbeatSequenceNumber(24093) adding peer(2)

Your example above is from the chat for me these lines are only in the actual server log and are missing in the server manager ui.

Maybe this all has to do with the deployment on my side, idk. It is very basic on a debian server in a docker container. I am using the Dockerfile from the repo.

As I said already, I also think it would be nice to have a switch to toggle this feature so that everybody can see the actual logs if needed.

But I have no problem if this is something only I need. Then I will just use it for my own.

@trigrab trigrab changed the base branch from master to develop August 26, 2020 14:44
@trigrab
Copy link
Author

trigrab commented Aug 26, 2020

I rebased my branch from develop and refactored my code. I did not understand how to use ReplaceAll to reduce complexity, but with ReplaceAllFunc it was very easy and reduced a lot of complexity of my code.

I would really appreciate, if you would look at it once again 🙂

@knoxfighter
Copy link
Member

knoxfighter commented Aug 28, 2020

ok, that makes sense and i like the addition now 😃

Since it is your first go program, this is something, that should be done with every go-code: run go fmt onto that source-file, so that your code gets automatically formatted to the go style guidelines (tab as intend, imports optimized, imports sorted, and more). When you use an IDE for go (GoLand) or an IDE with go plugins (VSC, atom), then they should run that automatically for you.

Since you replace the timestamps, that factorio uses to a timestamp, the information about the milliseconds get lost. Therefore i would like to see the ms too, so add them to your timestamp or keep the factorio-output in brackets.

Additional, should we implement that for the Cnosole too? I would say yes, but one after another. I would merge this PR and you create a new one for the Console 😉

src/gamelog.go Outdated

// set this to toggle log reformat of timestamps
var reformatTimestampsEnabled = true

func tailLog(filename string) ([]string, error) {
result := []string{}
Copy link
Member

Choose a reason for hiding this comment

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

replace empty slice with nil-slice
result := []string{} -> var result []string

src/gamelog.go Outdated

func reformatTimestamps(log []string) ([]string) {
getStartTime(log)
result := []string{}
Copy link
Member

Choose a reason for hiding this comment

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

same as above, replace empty slice to nil-slice

src/gamelog.go Outdated

func getStartTime(log []string) {
re, _ := regexp.Compile(`\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}`)
date := string(re.FindString(log[0]))
Copy link
Member

Choose a reason for hiding this comment

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

convertion from string to string not needed

src/gamelog.go Outdated
getStartTime(log)
result := []string{}

for i := range log {
Copy link
Member

Choose a reason for hiding this comment

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

use key-value for loop: for index, line := range log { or in this case for _, line := range log {, since you dont need the index.

@jannaahs jannaahs added this to the 1.0 milestone Jan 8, 2021
@knoxfighter knoxfighter removed this from the 1.0 milestone Jan 11, 2021
@knoxfighter knoxfighter closed this Mar 9, 2021
@knoxfighter knoxfighter deleted the branch OpenFactorioServerManager:develop March 9, 2021 20:38
@knoxfighter knoxfighter reopened this Mar 9, 2021
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