-
Notifications
You must be signed in to change notification settings - Fork 971
feat(sqlcommenter): add comment_position option to control SQL comment placement #4628
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| `opentelemetry-instrumentation`: Add `comment_position` option to `_add_sql_comment()` to support prepending sqlcommenter comment to the beginning of the query |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,17 +5,26 @@ | |
| from opentelemetry.instrumentation.utils import _url_quote | ||
|
|
||
|
|
||
| def _add_sql_comment(sql, **meta) -> str: | ||
| def _add_sql_comment(sql, comment_position="end", **meta) -> str: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pardon my delayed review @RiyaChaturvedi37 and thank you for resuming this! I had this comment on the other PR: Instead of a string, what if we had it as a case-insensitive boolean string check for false/true and the config was called |
||
| """ | ||
| Appends comments to the sql statement and returns it | ||
| Adds comments to the sql statement and returns it. | ||
| By default, the comment is appended to the end of the query. | ||
| Set comment_position="start" to prepend the comment instead. | ||
| """ | ||
| meta.update(**_add_framework_tags()) | ||
| comment = _generate_sql_comment(**meta) | ||
| if not comment: | ||
| return sql | ||
| sql = sql.rstrip() | ||
| if sql.endswith(";"): | ||
| sql = sql[:-1] + comment + ";" | ||
| sql = sql[:-1] | ||
| end = ";" | ||
| else: | ||
| sql = sql + comment | ||
| end = "" | ||
| if comment_position == "start": | ||
| sql = comment + " " + sql + end | ||
| else: | ||
| sql = sql + comment + end | ||
| return sql | ||
|
|
||
|
|
||
|
|
@@ -25,10 +34,8 @@ def _generate_sql_comment(**meta) -> str: | |
| **meta kwargs. | ||
| """ | ||
| key_value_delimiter = "," | ||
|
|
||
| if not meta: # No entries added. | ||
| return "" | ||
|
|
||
| # Sort the keywords to ensure that caching works and that testing is | ||
| # deterministic. It eases visual inspection as well. | ||
| return ( | ||
|
|
@@ -46,7 +53,6 @@ def _add_framework_tags() -> dict: | |
| """ | ||
| Returns orm related tags if any set by the context | ||
| """ | ||
|
|
||
| sqlcommenter_framework_values = ( | ||
| context.get_value("SQLCOMMENTER_ORM_TAGS_AND_VALUES") | ||
| if context.get_value("SQLCOMMENTER_ORM_TAGS_AND_VALUES") | ||
|
|
||
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.
Please could test coverage be added in https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/opentelemetry-instrumentation/tests/test_utils.py or a new file.