Skip to content
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

feat(datadog): add source type name to datadog #3342

Merged
merged 17 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion api/integrations/datadog/datadog.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,22 @@
logger = logging.getLogger(__name__)

EVENTS_API_URI = "api/v1/events"
FLAGSMITH_SOURCE_TYPE_NAME = "flagsmith"


class DataDogWrapper(AbstractBaseEventIntegrationWrapper):
def __init__(self, base_url: str, api_key: str, session: requests.Session = None):
def __init__(
self,
base_url: str,
api_key: str,
session: requests.Session = None,
use_custom_source: bool = False,
) -> None:
self.base_url = base_url
if self.base_url[-1] != "/":
self.base_url += "/"
self.events_url = f"{self.base_url}{EVENTS_API_URI}"
self.use_custom_source = use_custom_source

self.api_key = api_key
self.session = session or requests.Session()
Expand All @@ -34,6 +42,9 @@ def generate_event_data(audit_log_record: AuditLog) -> dict:
}

def _track_event(self, event: dict) -> None:
if self.use_custom_source:
event["source_type_name"] = FLAGSMITH_SOURCE_TYPE_NAME

response = self.session.post(
f"{self.events_url}?api_key={self.api_key}", data=json.dumps(event)
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 3.2.23 on 2024-01-29 15:54

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('datadog', '0003_datadogconfiguration_deleted_at'),
]

operations = [
migrations.AddField(
model_name='datadogconfiguration',
name='use_custom_source',
field=models.BooleanField(default=False),
),
]
2 changes: 2 additions & 0 deletions api/integrations/datadog/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,5 @@ class DataDogConfiguration(IntegrationsModel):
Project, on_delete=models.CASCADE, related_name="data_dog_config"
)
base_url = models.URLField(blank=False, null=False)

use_custom_source = models.BooleanField(default=False)
2 changes: 1 addition & 1 deletion api/integrations/datadog/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
class DataDogConfigurationSerializer(BaseProjectIntegrationModelSerializer):
class Meta:
model = DataDogConfiguration
fields = ("id", "base_url", "api_key")
fields = ("id", "base_url", "api_key", "use_custom_source")
43 changes: 35 additions & 8 deletions api/tests/unit/integrations/datadog/test_unit_datadog.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import json

import pytest
from pytest_mock import MockerFixture

from audit.models import AuditLog
from environments.models import Environment
from integrations.datadog.datadog import EVENTS_API_URI, DataDogWrapper
from integrations.datadog.datadog import (
EVENTS_API_URI,
FLAGSMITH_SOURCE_TYPE_NAME,
DataDogWrapper,
)


@pytest.mark.parametrize(
Expand All @@ -19,28 +24,50 @@ def test_datadog_initialized_correctly(base_url, expected_events_url):
api_key = "123key"

# When initialized
data_dog = DataDogWrapper(base_url=base_url, api_key=api_key)
data_dog = DataDogWrapper(
base_url=base_url, api_key=api_key, use_custom_source=True
)
Comment on lines +27 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes me think that use_custom_source defaulting to False is kinda backwards, isn't it? Shouldn't we always default to using flagsmith as the source for our very own wrapper around data dog?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the issue here is backwards compatibility. If existing customers that are using the datadog integration have filtering rules set up based on the previous (default) source type, then releasing this for all users would break those rules. I've added the default as False, then changed it at the model level to True so that all new instances get the custom source type name.


# Then
assert data_dog.events_url == expected_events_url
assert data_dog.use_custom_source is True


def test_datadog_track_event(mocker):
@pytest.mark.parametrize(
"event_data, use_custom_source, expected_data",
(
({"foo": "bar"}, False, {"foo": "bar"}),
(
{"foo": "bar"},
True,
{"foo": "bar", "source_type_name": FLAGSMITH_SOURCE_TYPE_NAME},
),
),
)
def test_datadog_track_event(
mocker: MockerFixture,
event_data: dict,
use_custom_source: bool,
expected_data: dict,
) -> None:
# Given
base_url = "https://test.com"
api_key = "key"
mock_session = mocker.MagicMock()

datadog = DataDogWrapper(base_url=base_url, api_key=api_key, session=mock_session)

event = {"foo": "bar"}
datadog = DataDogWrapper(
base_url=base_url,
api_key=api_key,
session=mock_session,
use_custom_source=use_custom_source,
)

# When
datadog._track_event(event)
datadog._track_event(event_data)

# Then
mock_session.post.assert_called_once_with(
f"{datadog.events_url}?api_key={api_key}", data=json.dumps(event)
f"{datadog.events_url}?api_key={api_key}", data=json.dumps(expected_data)
)


Expand Down
18 changes: 16 additions & 2 deletions api/tests/unit/integrations/datadog/test_unit_datadog_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ def test_should_create_datadog_config_when_post(
project: Project,
) -> None:
# Given
data = {"base_url": "http://test.com", "api_key": "abc-123"}
data = {
"base_url": "http://test.com",
"api_key": "abc-123",
"use_custom_source": True,
}
url = reverse("api-v1:projects:integrations-datadog-list", args=[project.id])
# When
response = admin_client.post(
Expand All @@ -26,6 +30,11 @@ def test_should_create_datadog_config_when_post(
assert response.status_code == status.HTTP_201_CREATED
assert DataDogConfiguration.objects.filter(project=project).count() == 1

created_config = DataDogConfiguration.objects.filter(project=project).first()
assert created_config.base_url == data["base_url"]
assert created_config.api_key == data["api_key"]
assert created_config.use_custom_source == data["use_custom_source"]


def test_should_return_400_when_duplicate_datadog_config_is_posted(
admin_client: APIClient,
Expand Down Expand Up @@ -94,7 +103,12 @@ def test_should_return_datadog_config_list_when_requested(
# Then
assert response.status_code == status.HTTP_200_OK
assert response.data == [
{"api_key": config.api_key, "base_url": config.base_url, "id": config.id}
{
"api_key": config.api_key,
"base_url": config.base_url,
"id": config.id,
"use_custom_source": False,
}
]


Expand Down
5 changes: 3 additions & 2 deletions docs/docs/integrations/apm/datadog.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ The second type of integration allows you to send Flag change events in Flagsmit
4. Add the Datadog URL into Flagsmith as the Base URL - (This is either `https://api.datadoghq.com/` or
`https://api.datadoghq.eu/`)

![Datadog](/img/integrations/datadog/datadog-1.png)
### Custom Source

![Datadog](/img/integrations/datadog/datadog-2.png)
Comment on lines -45 to -47
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were old, stale, images that I think are not necessary. The documentation is clear enough and these images will always become stale over time.

By checking the 'Use Custom Source' option, we will send all events with the source 'Flagsmith'. Leaving this unchecked
will mean events are labelled with the default 'My apps' source.

Flag change events will now be sent to Datadog.

Expand Down
Binary file removed docs/static/img/integrations/datadog/datadog-1.png
Binary file not shown.
Binary file removed docs/static/img/integrations/datadog/datadog-2.png
Binary file not shown.
42 changes: 42 additions & 0 deletions frontend/web/components/base/forms/Checkbox.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import React, { useRef } from 'react'
import ReactMarkdown from 'react-markdown'
import Icon from 'components/Icon'
import Utils from 'common/utils/utils'

interface CheckboxProps {
label: string
onChange: (value: boolean) => void
checked: boolean
id?: string
}

const Checkbox: React.FC<CheckboxProps> = ({
checked,
id,
label,
onChange,
}) => {
const idRef = useRef(id || Utils.GUID())
const handleChange = () => {
onChange(!checked)
}

return (
<>
<input id={idRef.current} type='checkbox' />
<label
onClick={handleChange}
className='mb-0'
htmlFor={idRef.current}
style={{ display: 'inline' }}
>
<span className='checkbox mr-2'>
{checked && <Icon name='checkmark-square' />}
</span>
{label}
</label>
</>
)
}

export default Checkbox
22 changes: 22 additions & 0 deletions frontend/web/components/base/forms/Input.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import MaskedInput from 'react-maskedinput'
import cn from 'classnames'
import Icon from 'components/Icon'
import Radio from './Radio'
import Checkbox from './Checkbox'

const maskedCharacters = {
'a': {
Expand Down Expand Up @@ -105,6 +107,26 @@ const Input = class extends React.Component {
sizeClassNames[size],
)

if (this.props.type === 'checkbox') {
return (
<Checkbox
label={this.props.label}
value={this.props.value}
onChange={this.props.onChange}
checked={!!this.props.value}
/>
)
} else if (this.props.type === 'radio') {
return (
<Radio
label={this.props.label}
value={this.props.value}
onChange={this.props.onChange}
checked={!!this.props.value}
/>
)
}

return (
<div className={className}>
{mask ? (
Expand Down
13 changes: 10 additions & 3 deletions frontend/web/components/modals/CreateEditIntegrationModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const CreateEditIntegration = class extends Component {
const handleOauthSignature = (res, isProject) => {
const signature = res && res.signature
if (signature) {
q
const postfix = `?redirect_url=${encodeURIComponent(
`${document.location.href}?environment=${this.state.data.flagsmithEnvironment}&configure=${this.props.id}`,
)}&signature=${signature}`
Expand Down Expand Up @@ -217,10 +218,16 @@ const CreateEditIntegration = class extends Component {
<Input
id={field.label.replace(/ /g, '')}
ref={(e) => (this.input = e)}
value={this.state.data[field.key]}
onChange={(e) => this.update(field.key, e)}
value={
typeof this.state.data[field.key] === 'undefined'
? this.state.data[field.key]
: field.default
}
onChange={(e) => {
this.update(field.key, e)
}}
isValid={!!this.state.data[field.key]}
type={field.hidden ? 'password' : 'text'}
type={field.hidden ? 'password' : field.inputType || 'text'}
className='full-width mb-2'
/>
)}
Expand Down
32 changes: 8 additions & 24 deletions frontend/web/components/pages/HomePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import data from 'common/data/base/_data'
import GoogleButton from 'components/GoogleButton'
import ConfigProvider from 'common/providers/ConfigProvider'
import Constants from 'common/constants'
import Icon from 'components/Icon'
import ErrorMessage from 'components/ErrorMessage'
import Button from 'components/base/forms/Button'
import { informationCircleOutline } from 'ionicons/icons'
import { IonIcon } from '@ionic/react'
import Checkbox from 'components/base/forms/Checkbox'

let controller = new AbortController()

Expand Down Expand Up @@ -637,37 +637,21 @@ const HomePage = class extends React.Component {
'mailing_list',
) && (
<div>
<input
onChange={(e) => {
API.setCookie(
'marketing_consent_given',
`${e.target.checked}`,
)
<Checkbox
label={
'Yes, I would like to signup for the twice monthly newsletter (optional)'
}
onChange={() => {
this.setState({
marketing_consent_given:
e.target.checked,
!this.state
.marketing_consent_given,
})
}}
id='mailinglist'
type='checkbox'
checked={
this.state.marketing_consent_given
}
/>
<label
className='mb-0'
htmlFor='mailinglist'
style={{ display: 'inline' }}
>
<span className='checkbox mr-2'>
{this.state
.marketing_consent_given && (
<Icon name='checkmark-square' />
)}
</span>
Yes, I would like to signup for the
twice monthly newsletter (optional)
</label>
</div>
)}
<div className='form-cta'>
Expand Down
Loading