DataDog / datadogpy

The Datadog Python library
https://datadoghq.com/
Other
609 stars 302 forks source link

Positional argument names of DogStatsd.event, ThreadStats.event differ in 0.43.0 #707

Closed aaronlutz closed 2 years ago

aaronlutz commented 2 years ago

Describe the bug

To Reproduce

$ pip install --upgrade datadog==0.42.0
$ touch alerting.py

In alerting.py

import os
import datadog

datadog.initialize(
    api_key=os.environ.get('DATADOG_API_KEY'),
    app_key=os.environ.get('DATADOG_APP_KEY'),
)

if os.environ.get('DATADOG_USE_DOGSTATSD'):
    datadog = datadog.DogStatsd()
else:
    datadog = datadog.ThreadStats()
    datadog.start()
$ DATADOG_API_KEY=foo DATADOG_APP_KEY=bar python
>>> import alerting
>>> alerting.datadog.event(
...    title='EventTitle',
...    text= 'EventBody',
...    alert_type='info',
... )
$ DATADOG_API_KEY=foo DATADOG_APP_KEY=bar DATADOG_USE_DOGSTATSD=true python
>>> import alerting
>>> alerting.datadog.event(
...    title='EventTitle',
...    text= 'EventBody',
...    alert_type='info',
... )
$ pip install --upgrade datadog==0.43.0
$ DATADOG_API_KEY=foo DATADOG_APP_KEY=bar python
>>> import alerting
>>> alerting.datadog.event(
...    title='EventTitle',
...    text= 'EventBody',
...    alert_type='info',
... )
$ DATADOG_API_KEY=foo DATADOG_APP_KEY=bar DATADOG_USE_DOGSTATSD=true python
>>> import alerting
>>> alerting.datadog.event(
...    title='EventTitle',
...    text= 'EventBody',
...    alert_type='info',
... )
TypeError: event() got an unexpected keyword argument 'text'

Expected behavior

Given the same *args and **kwargs, datadog.DogStatsd.event() and datadog.ThreadStats.event() should be interchangeable.

Environment and Versions (please complete the following information):

aaronlutz commented 2 years ago

I don't have sufficient repository permissions to apply the backward-incompatible label or resource/dogstatsd label.

github-actions[bot] commented 2 years ago

Thanks for your contribution!

This issue has been automatically marked as stale because it has not had activity in the last 30 days. Note that the issue will not be automatically closed, but this notification will remind us to investigate why there's been inactivity. Thank you for participating in the Datadog open source community.

If you would like this issue to remain open:

  1. Verify that you can still reproduce the issue in the latest version of this project.

  2. Comment that the issue is still reproducible and include updated details requested in the issue template.

jenn-parker commented 2 years ago

Ran into this today when we updated from 0.42.0 to 0.43.0. Changing the text param to message solved it, but this seems like kind of a silly backwards-incompatible change.

Our basic usage:

import os

from datadog.dogstatsd.base import DogStatsd

statsd = DogStatsd(
    host=os.getenv('DATADOG_HOST', ''),
    namespace='sample-service',
    constant_tags=[
        'application:sample-service',
        'environment:{}'.format(os.getenv('ENVIRONMENT', ''))
    ],
)

def process() -> None:
    statsd.event(title='Sample service process', text='started')

It appears this was changed in PR #670

sgnn7 commented 2 years ago

Hey @aaronlutz / @Jltp1972, Apologies for the delay on answering this. The reason for the change was that it did not align to service_check variable naming so text was changed to message and it was not flagged as backwards-incompatible because it is/was a positional argument. Given your feedback about your use cases via named arguments for positional args, the second assumption did not seem to hold fully for all users so this will be something that we'll be more careful in statsd package going forward. With that said and the length of time the release has been out, the impact seems relatively low to the users other than ensuring that ThreadStats API matches in the future so I will see about opening a PR for that for the next release.

sgnn7 commented 2 years ago

@aaronlutz / @Jltp1972 Consistency fix is merged so I expect 0.43.1+ to have it. In the meantime, you should be able to remove the explicit names on the title and text/message positional parameters for interchangeability between the two packages.