Skip to content

refactor: restructing instrumentation format#244

Merged
tswast merged 9 commits intogoogleapis:masterfrom
aravinsiva:clean-up-for-instrumentation
Sep 11, 2020
Merged

refactor: restructing instrumentation format#244
tswast merged 9 commits intogoogleapis:masterfrom
aravinsiva:clean-up-for-instrumentation

Conversation

@aravinsiva
Copy link
Contributor

@aravinsiva aravinsiva commented Aug 26, 2020

Reformatting instrumentation to a cleaner implementation.

Fixes #245 🦕

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Aug 26, 2020
@aravinsiva aravinsiva changed the title restructing instrumentation format feat: restructing instrumentation format Aug 26, 2020
@tswast tswast added the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 26, 2020
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. labels Aug 26, 2020
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label Aug 27, 2020
@tswast
Copy link
Contributor

tswast commented Aug 27, 2020

Nit on commit name: feat: restructing instrumentation format implies this is a new feature, but it's actually a refactoring of an existing feature. refactor: would be better since this doesn't actually change any behavior. See https://github.com/googleapis/release-please/blob/46d9d834abd6d11000476c6938b1b0e897c89b14/src/releasers/python.ts#L29-L42 for list of types we use.

@tswast tswast closed this Aug 27, 2020
@tswast tswast reopened this Aug 27, 2020
@aravinsiva aravinsiva changed the title feat: restructing instrumentation format refactor: restructing instrumentation format Aug 27, 2020
@tswast tswast added the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 27, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 27, 2020
name=span_name, attributes=span_attributes, client=self, job_ref=job_ref
):
return call()
return call()
Copy link
Contributor

@tswast tswast Aug 27, 2020

Choose a reason for hiding this comment

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

Could you add a unit test just for _call_api itself to check this code branch? It can verify that create_span is not called if span_name is not specified.

Edit: and maybe check that call() or self._connection.api_request is called exactly once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

test's added in test_client.py seen on line 253 -> 299

@tswast tswast added the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 27, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 27, 2020
@tswast tswast added the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 27, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 27, 2020
@plamut plamut added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 31, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 31, 2020
@tswast tswast added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 8, 2020
@google-cla
Copy link

google-cla bot commented Sep 8, 2020

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Sep 8, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 8, 2020
@tswast tswast added the kokoro:run Add this label to force Kokoro to re-run the tests. label Sep 11, 2020
@google-cla
Copy link

google-cla bot commented Sep 11, 2020

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@tswast tswast added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Sep 11, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Sep 11, 2020
@tswast tswast merged commit 240d359 into googleapis:master Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigquery Issues related to the googleapis/python-bigquery API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clean up instrumentation

5 participants