fix(kafka): pass *args, **kwargs to TracedConsumer.init #5887

rdghosal · 2023-05-17T17:34:25Z

Pass arbitrary positional and keyword arguments to TracedConsumer so as to allow client to configure their instance of KafkaConsumer. This is, for example, required for configuring a logger internal to the Consumer, where a logger can only be passed as a kwarg.

Currently, passing such a kwarg leads to the following error:
TypeError: TracedConsumer.__init__() got an unexpected keyword argument 'logger'

Related to issue #5837

Checklist

Change(s) are motivated and described in the PR description.
Testing strategy is described if automated tests are not included in the PR.
Risk is outlined (performance impact, potential for breakage, maintainability, etc).
Change is maintainable (easy to change, telemetry, documentation).
Library release note guidelines are followed.
Documentation is included (in-code, generated user docs, public corp docs).
OPTIONAL: PR description includes explicit acknowledgement of the performance implications of the change as reported in the benchmarks PR comment.

Reviewer Checklist

Title is accurate.
No unnecessary changes are introduced.
Description motivates each change.
Avoids breaking API changes unless absolutely necessary.
Testing strategy adequately addresses listed risk(s).
Change is maintainable (easy to change, telemetry, documentation).
Release note makes sense to a user of the library.
Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.

Yun-Kim

Hi @rdghosal, thanks for the fix! Let me know if you can add a regression test as well to make sure #5837 is fixed, otherwise I'm happy to add it for you 😄

rdghosal · 2023-05-17T20:56:57Z

Hi @Yun-Kim ! Thanks for reviewing this. Would it be possible to ask you to add these regression tests to the PR? I believe you should be able to tack on changes (let me know if not).

Yun-Kim

Thanks again for the contribution @rdghosal! 🥳

rdghosal · 2023-05-17T22:17:52Z

@Yun-Kim Thanks for adding that test! Will be sure to follow your example moving forward. Also, I see there are some failing checks. Please let me know if there's something I should contribute to get these passing.

mabdinur

left a few suggestions but aside from that LGTM

tests/contrib/kafka/test_kafka.py

releasenotes/notes/pass-args-kwargs-to-kafka-f3ca8128a2c4d612.yaml

Pass arbitrary positional and keyword arguments to `TracedConsumer` so as to allow client to configure their instance of `KafkaConsumer`. This is, for example, [required for configuring a logger](https://docs.confluent.io/platform/current/clients/confluent-kafka-python/html/index.html#kafka-client-configuration) internal to the Consumer, where a logger can only be passed as a kwarg. Currently, passing such a kwarg leads to the following error: ```TypeError: TracedConsumer.__init__() got an unexpected keyword argument 'logger'``` Related to issue [#5837](#5873) --- ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] OPTIONAL: PR description includes explicit acknowledgement of the performance implications of the change as reported in the benchmarks PR comment. ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. --------- Co-authored-by: Yun Kim <yun.kim@datadoghq.com> Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com> Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>

…5887 to 1.13] (#5901) Backports #5887 to 1.13. Pass arbitrary positional and keyword arguments to `TracedConsumer` so as to allow client to configure their instance of `KafkaConsumer`. This is, for example, [required for configuring a logger](https://docs.confluent.io/platform/current/clients/confluent-kafka-python/html/index.html#kafka-client-configuration) internal to the Consumer, where a logger can only be passed as a kwarg. Currently, passing such a kwarg leads to the following error: ```TypeError: TracedConsumer.__init__() got an unexpected keyword argument 'logger'``` Related to issue [#5837](#5873) --- ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] OPTIONAL: PR description includes explicit acknowledgement of the performance implications of the change as reported in the benchmarks PR comment. ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. Co-authored-by: Rahul D. Ghosal <RDGhosal@gmail.com> Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com> Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>

…5887 to 1.12] (#5902) Backports #5887 to 1.12 Pass arbitrary positional and keyword arguments to `TracedConsumer` so as to allow client to configure their instance of `KafkaConsumer`. This is, for example, [required for configuring a logger](https://docs.confluent.io/platform/current/clients/confluent-kafka-python/html/index.html#kafka-client-configuration) internal to the Consumer, where a logger can only be passed as a kwarg. Currently, passing such a kwarg leads to the following error: ```TypeError: TracedConsumer.__init__() got an unexpected keyword argument 'logger'``` Related to issue [#5837](#5873) --- ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] OPTIONAL: PR description includes explicit acknowledgement of the performance implications of the change as reported in the benchmarks PR comment. ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. Co-authored-by: Rahul D. Ghosal <RDGhosal@gmail.com> Co-authored-by: Munir Abdinur <munir_abdinur@hotmail.com> Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>

Blocks the release of v1.12.9, v1.13.4, and 1.14.0 Introduced by: #5887 ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). ## Reviewer Checklist - [ ] Title is accurate. - [ ] No unnecessary changes are introduced. - [ ] Description motivates each change. - [ ] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [ ] Testing strategy adequately addresses listed risk(s). - [ ] Change is maintainable (easy to change, telemetry, documentation). - [ ] Release note makes sense to a user of the library. - [ ] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.

Blocks the release of v1.12.9, v1.13.4, and 1.14.0 Introduced by: #5887 ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). ## Reviewer Checklist - [ ] Title is accurate. - [ ] No unnecessary changes are introduced. - [ ] Description motivates each change. - [ ] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [ ] Testing strategy adequately addresses listed risk(s). - [ ] Change is maintainable (easy to change, telemetry, documentation). - [ ] Release note makes sense to a user of the library. - [ ] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. (cherry picked from commit fb42173)

… to 1.12] (#5934) Backport of #5931 to 1.12 Blocks the release of v1.12.9, v1.13.4, and 1.14.0 Introduced by: #5887 ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.

… to 1.13] (#5933) Backport of #5931 to 1.13 Blocks the release of v1.12.9, v1.13.4, and 1.14.0 Introduced by: #5887 ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.

… to 1.14] (#5932) Backport of #5931 to 1.14 Blocks the release of v1.12.9, v1.13.4, and 1.14.0 Introduced by: #5887 ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.

fix(kafka): pass *args, **kwargs to TracedConsumer

706b7b2

rdghosal requested a review from a team as a code owner May 17, 2023 17:34

rdghosal requested review from Yun-Kim and mabdinur May 17, 2023 17:34

Rahul D. Ghosal added 2 commits May 17, 2023 13:34

Merge branch '1.x' into patch-1

88e9493

Merge branch '1.x' into patch-1

e484e2b

Yun-Kim reviewed May 17, 2023

View reviewed changes

Add regression test

ce24a68

Yun-Kim previously approved these changes May 17, 2023

View reviewed changes

release note

9bd2dd6

mabdinur dismissed Yun-Kim’s stale review via 9bd2dd6 May 17, 2023 22:35

mabdinur requested a review from a team as a code owner May 17, 2023 22:35

mabdinur reviewed May 17, 2023

View reviewed changes

tests/contrib/kafka/test_kafka.py Show resolved Hide resolved

releasenotes/notes/pass-args-kwargs-to-kafka-f3ca8128a2c4d612.yaml Outdated Show resolved Hide resolved

mabdinur and others added 3 commits May 17, 2023 18:37

ws

7a7ccf5

fmt

729bd31

comment

3949709

mabdinur approved these changes May 18, 2023

View reviewed changes

Merge branch '1.x' into patch-1

637065b

Yun-Kim enabled auto-merge (squash) May 18, 2023 14:32

Yun-Kim merged commit f5854aa into DataDog:1.x May 18, 2023

Yun-Kim mentioned this pull request May 18, 2023

fix(kafka): pass *args, **kwargs to TracedConsumer.__init__ [backport #5887 to 1.13] #5901

Merged

15 tasks

Yun-Kim mentioned this pull request May 18, 2023

fix(kafka): pass *args, **kwargs to TracedConsumer.__init__ [backport #5887 to 1.12] #5902

Merged

15 tasks

mabdinur mentioned this pull request May 22, 2023

docs: fix formatting and spelling in kafka release note #5931

Merged

14 tasks

This was referenced May 22, 2023

docs: fix formatting and spelling in kafka release note [backport #5931 to 1.14] #5932

Merged

docs: fix formatting and spelling in kafka release note [backport #5931 to 1.13] #5933

Merged

mabdinur mentioned this pull request May 22, 2023

docs: fix formatting and spelling in kafka release note [backport #5931 to 1.12] #5934

Merged

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(kafka): pass *args, **kwargs to TracedConsumer.init #5887

fix(kafka): pass *args, **kwargs to TracedConsumer.init #5887

Uh oh!

rdghosal commented May 17, 2023 •

edited by Yun-Kim

Loading

Uh oh!

Yun-Kim left a comment

Uh oh!

rdghosal commented May 17, 2023

Uh oh!

Yun-Kim left a comment

Uh oh!

rdghosal commented May 17, 2023

Uh oh!

mabdinur left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix(kafka): pass *args, **kwargs to TracedConsumer.__init__ #5887

fix(kafka): pass *args, **kwargs to TracedConsumer.__init__ #5887

Uh oh!

Conversation

rdghosal commented May 17, 2023 • edited by Yun-Kim Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Reviewer Checklist

Uh oh!

Yun-Kim left a comment

Choose a reason for hiding this comment

Uh oh!

rdghosal commented May 17, 2023

Uh oh!

Yun-Kim left a comment

Choose a reason for hiding this comment

Uh oh!

rdghosal commented May 17, 2023

Uh oh!

mabdinur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix(kafka): pass *args, **kwargs to TracedConsumer.init #5887

fix(kafka): pass *args, **kwargs to TracedConsumer.init #5887

rdghosal commented May 17, 2023 •

edited by Yun-Kim

Loading