Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

enhance: cut tracing content as unicode for safety #3342

Merged
merged 6 commits into from
Mar 13, 2024

Conversation

seth-shi
Copy link
Contributor

@seth-shi seth-shi commented Mar 1, 2024

more screen string field contains invalid UTF-8 on gzip content
safeContent for httpRequestBody and httpResponseBody

net/gtrace/gtrace_content.go Outdated Show resolved Hide resolved
net/gtrace/gtrace_content.go Outdated Show resolved Hide resolved
net/gtrace/gtrace_content.go Outdated Show resolved Hide resolved
net/gclient/gclient_tracing_tracer.go Outdated Show resolved Hide resolved
@gqcn gqcn changed the title fix http client trace gzip content enhance: cuts tracing content as unicode for safe Mar 4, 2024
@gqcn gqcn changed the title enhance: cuts tracing content as unicode for safe enhance: cuts tracing content as unicode for safety Mar 4, 2024
@gqcn gqcn changed the title enhance: cuts tracing content as unicode for safety enhance: cut tracing content as unicode for safety Mar 4, 2024
@seth-shi
Copy link
Contributor Author

seth-shi commented Mar 4, 2024

tomorrow fix

@houseme
Copy link
Member

houseme commented Mar 5, 2024

建议测试一下比较大的内容上报,防止卡住,以及文件上传的

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


It is recommended to test the content comparison to prevent getting stuck, as well as the file upload.

@seth-shi
Copy link
Contributor Author

seth-shi commented Mar 5, 2024

建议测试一下内容比较的,防止卡住,以及文件上传的

OK,我加个大 body 测试,现在的测试失败是什么原因导致的?

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


It is recommended to test the content comparison to prevent stuck and file upload.

OK, I'll add a big body test. What's the reason for the current test failure?

@houseme
Copy link
Member

houseme commented Mar 5, 2024

建议测试一下内容比较的,防止卡住,以及文件上传的

OK,我加个大 body 测试,现在的测试失败是什么原因导致的?

你可以搜索一下issues的记录,有同学反馈过,卡住的情况

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


It is recommended to test the content comparison to prevent stuck and file upload.

OK, I'll add a big body test. What's the reason for the current test failure?

You can search the records of issues. Some students have reported that they are stuck.

@seth-shi
Copy link
Contributor Author

seth-shi commented Mar 5, 2024

建议测试一下内容比较的,防止卡住,以及文件上传的

OK,我加个大 body 测试,现在的测试失败是什么原因导致的?

你可以搜索一下issues的记录,有同学反馈过,卡住的情况

感觉如果判断是 HTTP 文件上传是不是直接不记录更好.

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


It is recommended to test the content comparison to prevent getting stuck, and the file upload.

OK, I'll add a big body test. What's the reason for the current test failure?

You can search the records of issues. Some students have reported that they are stuck.

I feel that if it is judged to be an HTTP file upload, it would be better not to record it directly.

@houseme
Copy link
Member

houseme commented Mar 5, 2024

建议测试一下内容比较的,防止卡住,以及文件上传的

OK,我加个大 body 测试,现在的测试失败是什么原因导致的?

你可以搜索一下issues的记录,有同学反馈过,卡住的情况

感觉如果判断是 HTTP 文件上传是不是直接不记录更好.

我自己是没有碰到过这种问题,只有trace上报服务器配置错误,导致启动不起来的问题。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


It is recommended to test the content comparison to prevent stuck and file upload.

OK, I'll add a big body test. What's the reason for the current test failure?

You can search the records of issues. Some students have reported that they are stuck.

I feel that if it is judged as an HTTP file upload, it would be better not to record it directly.

I have never encountered this kind of problem myself. Only trace reported a server configuration error, which caused the server to fail to start.

@gqcn
Copy link
Member

gqcn commented Mar 12, 2024

@seth-shi If this pr is ready again, please AT me.

@seth-shi
Copy link
Contributor Author

seth-shi commented Mar 12, 2024

@gqcn

it`s already ok, is there anything need to fix ?

@gqcn gqcn merged commit a8713da into gogf:master Mar 13, 2024
23 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants