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

[collector] fix the wrong condition for checking the response #2067

Merged
merged 2 commits into from
Jun 15, 2024

Conversation

ileonli
Copy link
Contributor

@ileonli ileonli commented Jun 5, 2024

What's changed?

We should log the information only when the resp does not include text.

It appears that the condition used to check the response is incorrect.

Checklist

@@ -133,7 +133,7 @@ public void collect(CollectRep.MetricsData.Builder builder,
// Option 1: Parse using InputStream, but this requires significant code changes;
// Option 2: Manually trigger garbage collection, similar to how it's done in Dubbo for large inputs.
String resp = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
if (StringUtils.hasText(resp)) {
if (!StringUtils.hasText(resp)) {
log.info("http response entity is empty, status: {}.", statusCode);
Copy link
Member

Choose a reason for hiding this comment

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

When resp is empty, should we return directly without executing the subsequent logic?

Copy link
Contributor

Choose a reason for hiding this comment

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

some http request may not has response data themselves, but still need to be processed.

@tomsun28 tomsun28 merged commit bc4ddef into apache:master Jun 15, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants