Challengermode / CoreRcon

A .NET Standard implementation of the Source RCON Protocol.
MIT License
70 stars 12 forks source link

LogAddressPacket does not support milliseconds in Timestamp #46

Open Cryptoc1 opened 10 months ago

Cryptoc1 commented 10 months ago

Context: I'm building a custom log ingestion server (for use with logaddress_add_http in CS2), and am using the parsers in this library to handle certain logs being ingested.

Problem: The format of the log "packet" slightly differs when using HTTP:

As a result, I was unable to use the LogAddressPacket type for parsing log content.

Workaround:

I created the following LogPacket type in my project to handle these differences (note that I'm on net8.0):

public sealed partial record LogPacket( string Body, DateTime Timestamp )
{
    [GeneratedRegex( @"^(\d{2}/\d{2}/\d{4} - \d{2}:\d{2}:\d{2}\.\d{3}) - " )]
    private static partial Regex Parser( );

    public static bool TryParse( string? value, [NotNullWhen( true )] out LogPacket? packet )
    {
        if( !string.IsNullOrWhiteSpace( value ) )
        {
            var match = Parser().Match( value );
            if( match.Success && DateTime.TryParseExact( match.Groups[ 1 ].Value, "MM/dd/yyyy - HH:mm:ss.fff", CultureInfo.InvariantCulture, default, out var timestamp ) )
            {
                var body = value[ match.Groups[ 0 ].Length.. ].Trim();
                if( body.Length is not 0 )
                {
                    packet = new( body, timestamp );
                    return true;
                }
            }
        }

        packet = default;
        return false;
    }
}

The LogPacket.Body can then be used with DefaultParser<> implementations as expected.

I considered opening a PR, but wasn't sure if this use case fit the "scope" of this project.

xobust commented 10 months ago

Hey! I have not put much work into the Parsers recently. CS started diffing from the standard format and I didn't keep all parsers up to date. I also thought about moving parsers to a separate Project/Nugget package (#3). I do see some value in sharing some parsing code between RCON response parsing and Log parsing though.

Any contribution is welcome, and I will at least make sure that the parsers support the new timestamp format. Let me know if you want to discuss parsing improvements in greater detail. Challengermode has made some internal work parsing logs, and I would love to open source it in the future.

Cryptoc1 commented 3 months ago

@xobust Following up on this to let you know that I've addressed the issue on my v6-proposal fork, as well as #3, but I also got carried away and kind of rewrote the whole library (hence v6-propsal)... 😅

xobust commented 3 months ago

Hello again @Cryptoc1!

First of all, awsome work! I would love to assist in merging this, I am also happy to add you as a maintainer. There are some changes in the branch I have thought about making for a long time.

I have been adding some integration tests and making the RconClient an IAsyncDisposable. I think the Integreation tests are important as the RCON servers differ a little in implementation and the responses sometimes change (CS:GO & CS2 introduced a lot of changes to the standrad logs). If you have time we could have a call where we can discuss the changes. Alternativly you can add a pull request where I can add feedback.