Skip to content

Conversation

@AlchemyDing
Copy link
Member

resolve issue
When a large number of commands are sent to the Redis server at once using the RedisConnection.send(CommandsData data) method, a significant number of normalizeSingleCommand operations are executed. This can result in a long statement that takes up a large amount of memory or causes an Out-of-Memory (OOM) error. When users use batch execute, they usually do not pay attention to the specific content of each statement.

@AlchemyDing AlchemyDing requested a review from a team January 19, 2024 07:54
@AlchemyDing AlchemyDing changed the title Resolve Memory leak in redisson instrumentation Resolve memory leak in redisson instrumentation Jan 19, 2024
if (commandsData.getCommands().size() == 1) {
return commandsData.getCommands().get(0).getCommand().getName();
} else {
return "BATCH EXECUTE";
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the spec allow this? From what I understand from https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#call-level-attributes operation should not be set if there is more than one operation.

Copy link
Member Author

@AlchemyDing AlchemyDing Jul 29, 2025

Choose a reason for hiding this comment

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

db.operation.name
I noticed that there is a description of the db.operation.name for batch statements in semantic-conventions, but I'm not sure if this is the one adjusted after this pull request

default:
return String.join(";", sanitizedStatements);
}
return sanitizeStatement();
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of omitting the statement when multiple statements are executed you could have changed it to only capture some small number X statements or limit the total length of the statement (there is a size limit to attributes anyway). Did you consider solving it this way?

@laurit
Copy link
Contributor

laurit commented Jan 19, 2024

@AlchemyDing Just curious is this a guess that this change will solve the OOM or are you able to repro it?

@AlchemyDing AlchemyDing reopened this Jan 19, 2024
@AlchemyDing AlchemyDing marked this pull request as draft January 19, 2024 16:26
# Conflicts:
#	instrumentation/redisson/redisson-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonRequest.java
#	instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java
#	instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java
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.

Memory leak in redisson instrumentation

2 participants