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.

Sign up for GitHub

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

Jump to bottom

Feat: Add Mysql2 and Trilogy db.collection.name attribute #1109

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Feat: Add Mysql2 and Trilogy db.collection.name attribute #1109

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
43d24ee
Add Mysql2 and Trilogy collection_name attribute
hannahramadan Aug 8, 2024
8bd5818
Remove empty line
hannahramadan Aug 8, 2024
146bd71
Appease rubocop
hannahramadan Aug 8, 2024
f9c244e
Refactor compact!
hannahramadan Aug 20, 2024
6047052
Apply suggestions from code review
hannahramadan Aug 20, 2024
cc43b33
Merge branch 'main' into mysql_libs_db_collection_name
kaylareopelle Aug 20, 2024
b61e51e
Update to older semantic convention
hannahramadan Aug 21, 2024
bc7df18
Merge branch 'mysql_libs_db_collection_name' of https://github.com/ha…
hannahramadan Aug 21, 2024
a23b954
Update regex
hannahramadan Aug 26, 2024
f099af3
Use correct variable
hannahramadan Aug 26, 2024
8134ee3
Go directly to matching data vs MatchData object
hannahramadan Aug 26, 2024
8328668
Feature flag
hannahramadan Aug 27, 2024
8f42fa2
No nils
hannahramadan Aug 27, 2024
e73b23a
Omit db_sql_table by default
hannahramadan Oct 14, 2024
46616da
Capture table names in double quotes
hannahramadan Oct 14, 2024
4af3e2c
Don't report table name on error
hannahramadan Oct 30, 2024
d6ffcdf
Apply suggestions from code review
hannahramadan Nov 8, 2024
d9f1975
Update regex
hannahramadan Nov 8, 2024
b8d5a96
Merge branch 'mysql_libs_db_collection_name' of https://github.com/ha…
hannahramadan Nov 8, 2024
68f42ec
Only make one sql table method call
hannahramadan Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion instrumentation/mysql2/lib/opentelemetry/instrumentation/mysql2/patches/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def _otel_span_attributes(sql)
attributes = _otel_client_attributes(sql)

if sql
attributes[SemanticConventions::Trace::DB_SQL_TABLE] = db_sql_table_name(sql) if config[:db_sql_table] == :include
attributes[SemanticConventions::Trace::DB_SQL_TABLE] = db_sql_table_name(sql) if db_sql_table_name(sql) && config[:db_sql_table] == :include

case config[:db_statement]
when :include
Expand Down Expand Up @@ -98,6 +98,7 @@ def _otel_client_attributes(sql)
def db_sql_table_name(sql)
Regexp.last_match(1) if sql =~ TABLE_NAME
rescue StandardError
hannahramadan marked this conversation as resolved.
Show resolved Hide resolved
OpenTelemetry.handle_error(message: 'Error extracting collection name', exception: e)
nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

If an error occurs and the attribute is set to nil, then the SDK will report an error. It would be best to avoid setting any invalid attributes.

Please ensure that the attribute is not set in cases where the table name could not be extracted.

https://github.com/open-telemetry/opentelemetry-ruby/blob/555b062ef9421784c132aa9b97b29ec637b13b0f/sdk/lib/opentelemetry/sdk/trace/span.rb#L277

https://github.com/open-telemetry/opentelemetry-ruby/blob/555b062ef9421784c132aa9b97b29ec637b13b0f/sdk/lib/opentelemetry/sdk/internal.rb#L57

Copy link
Contributor Author

@hannahramadan hannahramadan Oct 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Made some changes! Will log an error if trouble getting the table name and won't report. Is the error logging here okay? 4af3e2c

end

Expand Down
Loading