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

fix issue when response contains ANSI escape sequences #38

Merged
merged 6 commits into from
Jul 5, 2024
Merged

Conversation

dmitryk-dk
Copy link
Contributor

@dmitryk-dk dmitryk-dk commented Jul 2, 2024

There is a parsing issue with ANSI escape sequences, which a standard JSON decoder can't handle.
I used valyala fastjson library to solve this issue. Victoria logs can handle logs like
{"_time":"2024-06-26T13:15:15.000Z","_stream_id":"00000000000000009eaf29866f70976a098adc735393deb1","_stream":"{compose_project=\"app\",compose_service=\"gateway\"}","_msg":"\x1b[2m2024-06-26T13:15:15.004Z\x1b[0;39m \x1b[32mTRACE\x1b[0;39m \x1b[35m1\x1b[0;39m \x1b[2m---\x1b[0;39m \x1b[2m[ parallel-19]\x1b[0;39m \x1b[36mo.s.c.g.f.WeightCalculatorWebFilter \x1b[0;39m \x1b[2m:\x1b[0;39m Weights attr: {} ","compose_project":"app","compose_service":"gateway"}

where \x1b[0;39m can be present.

Related issue: #24

@dmitryk-dk dmitryk-dk marked this pull request as ready for review July 3, 2024 10:03
Loori-R
Loori-R previously approved these changes Jul 4, 2024
@hagen1778
Copy link
Contributor

@dmitryk-dk do I understand it right that #24 is fixed on the vmlogs backend side and no changes in the plugin are required?

@dmitryk-dk
Copy link
Contributor Author

dmitryk-dk commented Jul 5, 2024

@dmitryk-dk do I understand it right that #24 is fixed on the vmlogs backend side and no changes in the plugin are required?

Yeah, I discussed the problem with @valyala, and he said that the main problem is on the Victorialogs side. But this PR helps to handle both situations with ANSI escape sequences and Unicode chars in the strings.
I have added tests for both situations, and they work well.
So, i think it would be helpful to have changes in our datasource

hagen1778
hagen1778 previously approved these changes Jul 5, 2024
@hagen1778 hagen1778 merged commit c7ba5e9 into main Jul 5, 2024
6 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.

3 participants