[mysqlreceiver]: rename mysql.commands metric to mysql.prepared_statements

[sumo-drosiek]

Description:

Rename mysql.commands metric to mysql.stmt_commands.

Actual mysql.commands is about prepared statement commands, and they shouldn't be mixed with regular commands as this will be messy and can be problematic in the future (e.g. reset is common command for both prepared statements and regular statements). In addition using mysql.commands metric is counterintuitive and unexpected for users

Link to tracking Issue:

#14138

Testing:
unit tests

Documentation: N/A

[sumo-drosiek]

@djaglowski @jpkrohling There can be possible conflict between Com_reset and Com_stmt_reset in the future as both of them will resolve to reset according to current approach

[djaglowski]

It's not clear to me that Com_xxx values should be combined with Com_stmt_xxx values. They are described here as quite separate categories. What do you think about adding these as a new metric?

[sumo-drosiek]

It's not clear to me that Com_xxx values should be combined with Com_stmt_xxx values.

This was my first iteration (first commit), but as this is breaking change and the names are not conflicting I decided to not do that. I will change it back

[sumo-drosiek]

I will issue adding proprt mysql_commands metric in separate PR after this is merged

[djaglowski]

Considering the Com_xxx vs Com_stmt_xxx difference, I agree the metric naming is less than ideal. However, I'm not sure this is really worth a breaking change at this point. I suspect this change will not be an obvious breaking change but a subtle one, which is worse in my opinion.

Can we just clarify the descriptions and add the Com_xxx values with a new metric name? Perhaps mysql.crud_commands?

[sumo-drosiek]

@djaglowski what do you think about another label, which will differ between statements and non_statements. I don't feel crud is covering all of possible metrics, which can be generated for commands. Maybe the other commands can be grouped into different metrics, but not sure if making such complication is good way too

list of possible metrics:

opentelemetry-collector-contrib/receiver/mysqlreceiver/testdata/scraper/global_stats.txt

Lines 11 to 181 in 6dc71bd

	Com_admin_commands 10
	Com_assign_to_keycache 11
	Com_alter_db 12
	Com_alter_event 13
	Com_alter_function 14
	Com_alter_instance 15
	Com_alter_procedure 16
	Com_alter_resource_group 17
	Com_alter_server 18
	Com_alter_table 19
	Com_alter_tablespace 20
	Com_alter_user 21
	Com_alter_user_default_role 22
	Com_analyze 23
	Com_begin 24
	Com_binlog 25
	Com_call_procedure 26
	Com_change_db 27
	Com_change_master 28
	Com_change_repl_filter 29
	Com_change_replication_source 30
	Com_check 31
	Com_checksum 32
	Com_clone 33
	Com_commit 34
	Com_create_db 35
	Com_create_event 36
	Com_create_function 37
	Com_create_index 38
	Com_create_procedure 39
	Com_create_role 40
	Com_create_server 41
	Com_create_table 42
	Com_create_resource_group 43
	Com_create_trigger 44
	Com_create_udf 45
	Com_create_user 46
	Com_create_view 47
	Com_create_spatial_reference_system 48
	Com_dealloc_sql 49
	Com_delete 50
	Com_delete_multi 51
	Com_do 52
	Com_drop_db 53
	Com_drop_event 54
	Com_drop_function 55
	Com_drop_index 56
	Com_drop_procedure 57
	Com_drop_resource_group 58
	Com_drop_role 59
	Com_drop_server 60
	Com_drop_spatial_reference_system 61
	Com_drop_table 62
	Com_drop_trigger 63
	Com_drop_user 64
	Com_drop_view 65
	Com_empty_query 66
	Com_execute_sql 67
	Com_explain_other 68
	Com_flush 69
	Com_get_diagnostics 70
	Com_grant 71
	Com_grant_roles 72
	Com_ha_close 73
	Com_ha_open 74
	Com_ha_read 75
	Com_help 76
	Com_import 77
	Com_insert 78
	Com_insert_select 79
	Com_install_component 80
	Com_install_plugin 81
	Com_kill 82
	Com_load 83
	Com_lock_instance 84
	Com_lock_tables 85
	Com_optimize 86
	Com_preload_keys 87
	Com_prepare_sql 88
	Com_purge 89
	Com_purge_before_date 90
	Com_release_savepoint 91
	Com_rename_table 92
	Com_rename_user 93
	Com_repair 94
	Com_replace 95
	Com_replace_select 96
	Com_reset 97
	Com_resignal 98
	Com_restart 99
	Com_revoke 100
	Com_revoke_all 101
	Com_revoke_roles 102
	Com_rollback 103
	Com_rollback_to_savepoint 104
	Com_savepoint 105
	Com_select 106
	Com_set_option 107
	Com_set_password 108
	Com_set_resource_group 109
	Com_set_role 110
	Com_signal 111
	Com_show_binlog_events 112
	Com_show_binlogs 113
	Com_show_charsets 114
	Com_show_collations 115
	Com_show_create_db 116
	Com_show_create_event 117
	Com_show_create_func 118
	Com_show_create_proc 119
	Com_show_create_table 120
	Com_show_create_trigger 121
	Com_show_databases 122
	Com_show_engine_logs 123
	Com_show_engine_mutex 124
	Com_show_engine_status 125
	Com_show_events 126
	Com_show_errors 127
	Com_show_fields 128
	Com_show_function_code 129
	Com_show_function_status 130
	Com_show_grants 131
	Com_show_keys 132
	Com_show_master_status 133
	Com_show_open_tables 134
	Com_show_plugins 135
	Com_show_privileges 136
	Com_show_procedure_code 137
	Com_show_procedure_status 138
	Com_show_processlist 139
	Com_show_profile 140
	Com_show_profiles 141
	Com_show_relaylog_events 142
	Com_show_replicas 143
	Com_show_slave_hosts 144
	Com_show_replica_status 145
	Com_show_slave_status 146
	Com_show_status 147
	Com_show_storage_engines 148
	Com_show_table_status 149
	Com_show_tables 150
	Com_show_triggers 151
	Com_show_variables 152
	Com_show_warnings 153
	Com_show_create_user 154
	Com_shutdown 155
	Com_replica_start 156
	Com_slave_start 157
	Com_replica_stop 158
	Com_slave_stop 159
	Com_group_replication_start 160
	Com_group_replication_stop 161
	Com_stmt_execute 162
	Com_stmt_close 163
	Com_stmt_fetch 164
	Com_stmt_prepare 165
	Com_stmt_reset 166
	Com_stmt_send_long_data 167
	Com_truncate 168
	Com_uninstall_component 169
	Com_uninstall_plugin 170
	Com_unlock_instance 171
	Com_unlock_tables 172
	Com_update 173
	Com_update_multi 174
	Com_xa_commit 175
	Com_xa_end 176
	Com_xa_prepare 177
	Com_xa_recover 178
	Com_xa_rollback 179
	Com_xa_start 180

This new label (kind probably) can be use to differ between stmt and other command types. However not sure if specification is happy for labels which do not apply with all other labels:

	stmt	basic
create	no	yes
prepare	yes	no

[djaglowski]

Unfortunately, I don't think an additional attribute works here for the reason you noted.

I can't argue that mysql.commands is a best name for the com_xxx commands, and that the com_stmt_xxx commands should use a different name (eg mysql.prepared_statements).

In the absence of a better suggestion, I think we just make the breaking change. The component is still in beta so this is possible still.

[sumo-drosiek]

@djaglowski should be ready to go

[djaglowski]

Looks like there's a code generation issue:

 | Name | Description | Type |
 | ---- | ----------- | ---- |
-| mysql.instance.endpoint | Endpoint of the MySQL instance. | String |
+| mysql.instance.endpoint | Endpoint of the MySQL instance. | Str |
 
 ## Metric attributes
 
Generated code is out of date, please run "make generate" and commit the changes in this PR.

[codeboten]

@sumo-drosiek I'd like to help move these from approved to merged, but there's a lot of PRs with many conflicts to resolve. Can you resolve conflicts of one PR at a time and ping me when they're resolved?

Resolving more than one will just result in a merge conflict once any of them is merged (i'm guessing since they're touching the same files)

In the future, it may be easier to either open one pr for all new metrics, or build each PR on top of the other one and leave it in draft until the previous PR is merged.

[sumo-drosiek]

@codeboten I see that issue as well, I would love to make PR chain, but as code is outside of this repo the PRs won't create here

In the future, it may be easier to either open one pr for all new metrics, or build each PR on top of the other one and leave it in draft until the previous PR is merged.

I can prepare one common PR for all approved PRs

[sumo-drosiek]

@codeboten This commit should be good to go

[codeboten]

Please take a look at the failing porto check:

porto -w --include-internal --skip-dirs "^cmd$" ./
diff --git a/receiver/mysqlreceiver/internal/metadata/deprecated_metrics_commands.go b/receiver/mysqlreceiver/internal/metadata/deprecated_metrics_commands.go
index 8ac87fe..45be9d1 100644
--- a/receiver/mysqlreceiver/internal/metadata/deprecated_metrics_commands.go
+++ b/receiver/mysqlreceiver/internal/metadata/deprecated_metrics_commands.go
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package metadata
+package metadata // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/mysqlreceiver/internal/metadata"
 
 // deprecateDescription add [DEPRECATED] prefix to description.
 func (m *metricMysqlCommands) deprecateDescription() {
Porto links are out of date, please run "make goporto" and commit the changes in this PR.
Error: Process completed with exit code 1.

[sumo-drosiek]

PR is ready to merge. Rebased and changed OTC version in warnings