-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[receiver/mysql] feat: add table size metrics #32680
Conversation
Is there a corresponding issue for this PR? |
No, I didn’t see an existing one. However, I've just created a new one: #32693 |
3c32f3d
to
47aa445
Compare
3bb8959
to
2027133
Compare
@djaglowski I've updated the PR to clean it up. Please review. Thank you |
4af2bc5
to
3d8aa8a
Compare
receiver/mysqlreceiver/metadata.yaml
Outdated
mysql.table.total_length: | ||
enabled: false | ||
description: The total length (sum of data_length and index_length) in bytes for a given table. | ||
unit: By | ||
sum: | ||
value_type: int | ||
monotonic: false | ||
aggregation_temporality: cumulative | ||
attributes: [table_name, schema] | ||
mysql.table.data_length: | ||
enabled: false | ||
description: The data length in bytes for a given table. | ||
unit: By | ||
sum: | ||
value_type: int | ||
monotonic: false | ||
aggregation_temporality: cumulative | ||
attributes: [table_name, schema] | ||
mysql.table.index_length: | ||
enabled: false | ||
description: The index length in bytes for a given table. | ||
unit: By | ||
sum: | ||
value_type: int | ||
monotonic: false | ||
aggregation_temporality: cumulative | ||
attributes: [table_name, schema] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just model this as one metric with two data points and an attribute which is either data or index? This is how OTel's data model is intended to work, rather than just returning sums as separate metrics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that makes sense. I'll remove the total_length
metric.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still have separate metrics for data and index length. The problem with this is that it is now difficult for users to combine the two in most backends. OTel typically will combine these into one metric and use an attribute to differentiate between the two. Any reason not to do this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, typically can we name this metric mysql.table.size
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I misunderstood your last comment. I'll have that changed. Thank you for the review!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still see two separate metrics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@djaglowski Please check now. Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@djaglowski Checking in - please let me now if there needs additional changes
b93c042
to
6cfa283
Compare
6cfa283
to
43feb9a
Compare
43feb9a
to
5437618
Compare
Description: This adds table size metrics using the INFORMATION_SCHEMA TABLES table: https://dev.mysql.com/doc/refman/8.3/en/information-schema-tables-table.html. Specifically, we are adding the columns:
TABLE_ROWS
,AVG_ROW_LENGTH
,DATA_LENGTH
,INDEX_LENGTH
Link to tracking issue: #32693
Testing: Added testing data, tested locally against MySQL 8.
Documentation: By default, the metric is turned off. When enabled, make sure that the MySQL user has access to the tables being monitored.