Add mariadb #2570

ZStriker19 · 2021-06-21T15:54:43Z

Integration

This PR adds support for mariadb.

Links

Integration docs:
- https://mariadb.com/docs/clients/mariadb-connectors/connector-python/
- https://mariadb-corporation.github.io/mariadb-connector-python/usage.html
Corp docs PR: add mariadb to compatibility documentation#10838

Added the PR to make changes to the test docker container. This will need to be merged before mariadb tests will pass in CircleCI: #2584

Checklist

…SAMPLING=false is set, not just when an older tracer version is being used

…egration

… __init__.py file for mariadb correct

ddtrace/__init__.py

ddtrace/contrib/mariadb/__init__.py

ddtrace/contrib/mariadb/patch.py

tests/contrib/mariadb/test_mariadb.py

tests/contrib/mysql/test_mysql.py

tests/snapshots/tests.contrib.mariadb.test_mariadb.test_simple_query_snapshot.snap

tests/contrib/mariadb/test_mariadb.py

P403n1x87 · 2021-06-25T14:13:32Z

ddtrace/contrib/mariadb/patch.py

+
+def _connect(func, instance, args, kwargs):
+    conn = func(*args, **kwargs)
+    ##need to pull from args as well at some point


We have the helper get_argument_value for this in ddtrace.utils

It looks like mariadb's connect function only supports kwargs: https://mariadb-corporation.github.io/mariadb-connector-python/module.html#mariadb.connect

Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>

…ching and unpatching, used black formatter

…_mariadb

ddtrace/contrib/mariadb/__init__.py

tests/contrib/mariadb/test_mariadb.py

tests/contrib/mysql/test_mysql.py

tests/contrib/config.py

Kyle-Verhoog · 2021-07-12T18:38:59Z

tests/snapshots/tests.contrib.mariadb.test_mariadb.test_query_proc_snapshot.snap

@@ -0,0 +1,22 @@
+[[{"name" "mariadb.query"
+   "service" "mariadb"
+   "resource" "sp_sum"


Interesting that the resource for a procedure ends up getting truncated to this... @brettlangdon any thoughts?

this is probably why:

dd-trace-py/ddtrace/contrib/dbapi/__init__.py

Line 115 in 0dc356f

return self._trace_method(self.__wrapped__.callproc, self._self_datadog_name, proc, {}, proc, *args)

because it is cursor.callproc("proc_name") we don't have the query, that is stored on the server

I am surprised we only have 1 span here since this snapshot tests has two executes and the callproc ?

@Kyle-Verhoog any further thoughts on this?

ah for some reason i didn't realize that sp_sum was the given name of the stored procedure.. seems totally fine to me 👍

I am surprised we only have 1 span here since this snapshot tests has two executes and the callproc ?

lol yeah the test for some reason disables the tracer for the two executes: https://github.com/DataDog/dd-trace-py/pull/2570/files#diff-b5666b23f77bb419456af9d9b0a778a1a787fafbee39303612e5de24e4960500R264

tests/utils.py

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

…_mariadb Pulling changes

ddtrace/contrib/mariadb/__init__.py

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

…_mariadb pulling

brettlangdon

previous comments I'd like to see resolved, otherwise lgtm

brettlangdon · 2021-07-20T14:45:36Z

tests/snapshots/tests.contrib.mariadb.test_mariadb.test_query_proc_snapshot.snap

@@ -0,0 +1,22 @@
+[[{"name" "mariadb.query"
+   "service" "mariadb"
+   "resource" "sp_sum"


@Kyle-Verhoog any further thoughts on this?

majorgreys

small nit

Dockerfile.buster

brettlangdon · 2021-07-20T17:45:07Z

tests/contrib/mariadb/test_mariadb.py

+
+@snapshot(include_tracer=True)
+def test_simple_query_snapshot(tracer):
+    with get_connection(tracer) as connection:


you could just use your connection fixture right? should do the same thing

ddtrace/contrib/mariadb/__init__.py

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

Kyle-Verhoog

Literally just one thing left and we're good to go!

Dockerfile.buster

Kyle-Verhoog

🙌 🙌 🙌

comments have been addressed

ZStriker19 and others added 15 commits April 19, 2021 13:08

adding comment to clarify that log line occurs when DATADOG_PRIORITY_…

39fad4f

…SAMPLING=false is set, not just when an older tracer version is being used

Update writer.py

9a22d05

got mariadb integration working on sample app

aed7bfe

testing for mariadb integration setup

2d74914

fixed riot.py file so mariadb tests run

a501d42

beginning conversion of mysql tests to mariadb tests with new formats

3f23a64

committing to merge in master updates

71b3506

Merge remote-tracking branch 'origin' into add_mariadb

cdc7aab

made all mariadb tests pass

b37d210

added first snapshot test

6001351

removed print statement and added more snapshot tests for mariadb int…

3e32f6f

…egration

updated docs to include mariadb integration and made documentation in…

bfd87e9

… __init__.py file for mariadb correct

Merge remote-tracking branch 'origin' into add_mariadb

0500961

added mariadb to circleci config.yaml

6c0d22a

added mariadb build steps to dev image

74e945a

ZStriker19 mentioned this pull request Jun 24, 2021

added mariadb build steps to dev image #2584

Merged

4 tasks

brettlangdon reviewed Jun 25, 2021

View reviewed changes

P403n1x87 reviewed Jun 25, 2021

View reviewed changes

ZStriker19 and others added 5 commits June 28, 2021 12:26

added malformed query snapshot test

cb8b2ab

Fix grammar in mariadb docs

158fff0

Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>

added mariadb test snapshots, reformatted tests, added guards for pat…

5330d3d

…ching and unpatching, used black formatter

Merge branch 'add_mariadb' of github.com:DataDog/dd-trace-py into add…

7999026

…_mariadb

Merge branch 'master' into add_mariadb

015adf8

Kyle-Verhoog self-requested a review July 8, 2021 16:39

added release notes

1f29742

Kyle-Verhoog requested changes Jul 12, 2021

View reviewed changes

ZStriker19 and others added 4 commits July 13, 2021 15:37

add link to library this instruments

2b8683a

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

remove comment

8d3e56b

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

Update ddtrace/contrib/mariadb/__init__.py

eac06e3

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

Update ddtrace/contrib/mariadb/__init__.py

489eddb

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

Merge branch 'add_mariadb' of github.com:DataDog/dd-trace-py into add…

1b5aae6

…_mariadb Pulling changes

Kyle-Verhoog reviewed Jul 19, 2021

View reviewed changes

ddtrace/contrib/mariadb/__init__.py Outdated Show resolved Hide resolved

ddtrace/contrib/mariadb/__init__.py Outdated Show resolved Hide resolved

ZStriker19 and others added 7 commits July 19, 2021 11:42

changing test case so tracer flushes

59ef781

changing test case to pass in CI

ff54feb

formatting

8ff94b4

Update ddtrace/contrib/mariadb/__init__.py

5d53d39

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

Update ddtrace/contrib/mariadb/__init__.py

cc74fc0

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

snapshots

83d5e01

Merge branch 'add_mariadb' of github.com:DataDog/dd-trace-py into add…

9172168

…_mariadb pulling

brettlangdon previously approved these changes Jul 20, 2021

View reviewed changes

majorgreys previously requested changes Jul 20, 2021

View reviewed changes

Dockerfile.buster Outdated Show resolved Hide resolved

updating tests so connection closes on each

3cd4893

ZStriker19 dismissed brettlangdon’s stale review via 3cd4893 July 20, 2021 17:33

Extend environment instead of replacing it

82f47fc

brettlangdon reviewed Jul 20, 2021

View reviewed changes

Kyle-Verhoog reviewed Jul 20, 2021

View reviewed changes

ddtrace/contrib/mariadb/__init__.py Outdated Show resolved Hide resolved

ZStriker19 and others added 3 commits July 20, 2021 11:54

fix formatting and refactor a bit in tests

e567e5f

Update ddtrace/contrib/mariadb/__init__.py

64dfb74

Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>

Merge branch 'master' into add_mariadb

e521fe4

Kyle-Verhoog reviewed Jul 20, 2021

View reviewed changes

Dockerfile.buster Outdated Show resolved Hide resolved

ZStriker19 added 2 commits July 20, 2021 20:47

formatting

c373a38

formatting

6baf6e7

Kyle-Verhoog approved these changes Jul 21, 2021

View reviewed changes

Kyle-Verhoog requested a review from majorgreys July 21, 2021 11:56

Merge branch 'master' into add_mariadb

10b19e5

brettlangdon approved these changes Jul 21, 2021

View reviewed changes

mergify bot merged commit c44c1b4 into master Jul 21, 2021

mergify bot deleted the add_mariadb branch July 21, 2021 13:21

Add mariadb #2570

Add mariadb #2570

Uh oh!

Conversation

ZStriker19 commented Jun 21, 2021 • edited by Kyle-Verhoog Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Integration

Links

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brettlangdon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

majorgreys left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kyle-Verhoog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kyle-Verhoog left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ZStriker19 commented Jun 21, 2021 •

edited by Kyle-Verhoog

Loading