Skip to content

feat(aws): Move tags enablement environment variable to settings #1004

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 6, 2025
Merged

feat(aws): Move tags enablement environment variable to settings #1004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion aws/logs_monitoring/caching/cloudwatch_log_group_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
DD_S3_CACHE_DIRNAME,
DD_S3_LOG_GROUP_CACHE_DIRNAME,
DD_TAGS_CACHE_TTL_SECONDS,
get_fetch_log_group_tags,
)
from telemetry import send_forwarder_internal_metrics

Expand Down Expand Up @@ -61,7 +62,7 @@ def get(self, log_group_arn):
return self._fetch_log_group_tags(log_group_arn)

def _should_fetch_tags(self):
return os.environ.get("DD_FETCH_LOG_GROUP_TAGS", "false").lower() == "true"
return get_fetch_log_group_tags()

def _fetch_log_group_tags(self, log_group_arn):
# first, check in-memory cache
Expand Down
5 changes: 2 additions & 3 deletions aws/logs_monitoring/caching/lambda_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import os

from botocore.exceptions import ClientError

from caching.base_tags_cache import BaseTagsCache
Expand All @@ -8,6 +6,7 @@
DD_S3_LAMBDA_CACHE_FILENAME,
DD_S3_LAMBDA_CACHE_LOCK_FILENAME,
GET_RESOURCES_LAMBDA_FILTER,
get_fetch_lambda_tags,
)
from telemetry import send_forwarder_internal_metrics

Expand All @@ -19,7 +18,7 @@ def __init__(self, prefix):
)

def should_fetch_tags(self):
return os.environ.get("DD_FETCH_LAMBDA_TAGS", "false").lower() == "true"
return get_fetch_lambda_tags()

def build_tags_cache(self):
"""Makes API calls to GetResources to get the live tags of the account's Lambda functions
Expand Down
7 changes: 4 additions & 3 deletions aws/logs_monitoring/caching/s3_tags_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import os
from botocore.exceptions import ClientError

from caching.base_tags_cache import BaseTagsCache
from caching.common import parse_get_resources_response_for_tags_by_arn
from telemetry import send_forwarder_internal_metrics
from settings import (
DD_S3_TAGS_CACHE_FILENAME,
DD_S3_TAGS_CACHE_LOCK_FILENAME,
GET_RESOURCES_S3_FILTER,
get_fetch_s3_tags,
)
from telemetry import send_forwarder_internal_metrics


class S3TagsCache(BaseTagsCache):
Expand All @@ -18,7 +19,7 @@ def __init__(self, prefix):

def should_fetch_tags(self):
# set it to true if we don't have the environment variable set to keep the default behavior
return os.environ.get("DD_FETCH_S3_TAGS", "true").lower() == "true"
return get_fetch_s3_tags()

def build_tags_cache(self):
"""Makes API calls to GetResources to get the live tags of the account's S3 buckets
Expand Down
9 changes: 5 additions & 4 deletions aws/logs_monitoring/caching/step_functions_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import os
from botocore.exceptions import ClientError

from caching.base_tags_cache import BaseTagsCache
from caching.common import (
sanitize_aws_tag_string,
parse_get_resources_response_for_tags_by_arn,
sanitize_aws_tag_string,
)
from telemetry import send_forwarder_internal_metrics
from settings import (
DD_S3_STEP_FUNCTIONS_CACHE_FILENAME,
DD_S3_STEP_FUNCTIONS_CACHE_LOCK_FILENAME,
GET_RESOURCES_STEP_FUNCTIONS_FILTER,
get_fetch_step_functions_tags,
)
from telemetry import send_forwarder_internal_metrics


class StepFunctionsTagsCache(BaseTagsCache):
Expand All @@ -22,7 +23,7 @@ def __init__(self, prefix):
)

def should_fetch_tags(self):
return os.environ.get("DD_FETCH_STEP_FUNCTIONS_TAGS", "false").lower() == "true"
return get_fetch_step_functions_tags()

def build_tags_cache(self):
"""Makes API calls to GetResources to get the live tags of the account's Step Functions
Expand Down
31 changes: 31 additions & 0 deletions aws/logs_monitoring/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,37 @@ def __init__(self, name, pattern, placeholder, enabled=True):
"DD_MULTILINE_LOG_REGEX_PATTERN", default=None
)

DD_FETCH_S3_TAGS = get_env_var("DD_FETCH_S3_TAGS", default="true", boolean=True)

DD_FETCH_LOG_GROUP_TAGS = get_env_var(
"DD_FETCH_LOG_GROUP_TAGS", default="true", boolean=True
)

DD_FETCH_LAMBDA_TAGS = get_env_var(
"DD_FETCH_LAMBDA_TAGS", default="false", boolean=True
)

DD_FETCH_STEP_FUNCTIONS_TAGS = get_env_var(
"DD_FETCH_STEP_FUNCTIONS_TAGS", default="false", boolean=True
)


def get_fetch_s3_tags():
Copy link
Contributor

Choose a reason for hiding this comment

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

imo, we could just import the setting directly in the cache function an use it. not a strict opinion, but it will be consistent with the other settings consts that we import.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was my first attempt, but with mocking involved, I needed to reload settings, then lambda_cache (for example), but lambda_cache was already patched with mocks and tests were deeply broken.

Using a getter allows me to only reload settings, and the rest of components will dynamically benefit from it.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I added this comment regarding consistency in settings vars imports. Otherwise, it's not really changing anything.

Regarding the patching, I think it's a workaround here to do getters. However, I think we could use a code refactor for reworking patching.

We can check that later

return DD_FETCH_S3_TAGS


def get_fetch_log_group_tags():
return DD_FETCH_LOG_GROUP_TAGS


def get_fetch_lambda_tags():
return DD_FETCH_LAMBDA_TAGS


def get_fetch_step_functions_tags():
return DD_FETCH_STEP_FUNCTIONS_TAGS


DD_SOURCE = "ddsource"
DD_CUSTOM_TAGS = "ddtags"
DD_SERVICE = "service"
Expand Down
46 changes: 26 additions & 20 deletions aws/logs_monitoring/tests/test_enhanced_lambda_metrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,34 @@
import json
import unittest
import os
import sys
import unittest
from importlib import reload
from time import time
from unittest.mock import patch, MagicMock
from unittest import mock
from unittest.mock import MagicMock, patch

from approvaltests.approvals import verify_as_json

from caching.lambda_cache import LambdaTagsCache
from enhanced_lambda_metrics import (
parse_metrics_from_report_log,
parse_metrics_from_json_report_log,
parse_lambda_tags_from_arn,
generate_enhanced_lambda_metrics,
create_out_of_memory_enhanced_metric,
generate_enhanced_lambda_metrics,
parse_lambda_tags_from_arn,
parse_metrics_from_json_report_log,
parse_metrics_from_report_log,
)

from caching.lambda_cache import LambdaTagsCache


class TestEnhancedLambdaMetrics(unittest.TestCase):
maxDiff = None
malformed_report = "REPORT invalid report log line"
standard_report = (
"REPORT RequestId: 8edab1f8-7d34-4a8e-a965-15ccbbb78d4c "
"Duration: 0.62 ms Billed Duration: 100 ms Memory Size: 128 MB Max Memory Used: 51 MB"
"REPORT RequestId: 8edab1f8-7d34-4a8e-a965-15ccbbb78d4c "
"Duration: 0.62 ms Billed Duration: 100 ms Memory Size: 128 MB Max Memory Used: 51 MB"
)
cold_start_report = (
"REPORT RequestId: 8edab1f8-7d34-4a8e-a965-15ccbbb78d4c "
"Duration: 0.81 ms Billed Duration: 100 ms Memory Size: 128 MB Max Memory Used: 90 MB Init Duration: 1234 ms"
"REPORT RequestId: 8edab1f8-7d34-4a8e-a965-15ccbbb78d4c "
"Duration: 0.81 ms Billed Duration: 100 ms Memory Size: 128 MB Max Memory Used: 90 MB Init Duration: 1234 ms"
)
report_with_xray = (
"REPORT RequestId: 814ba7cb-071e-4181-9a09-fa41db5bccad\tDuration: 1711.87 ms\t"
Expand Down Expand Up @@ -303,7 +305,10 @@ def test_generate_enhanced_lambda_metrics_once_with_missing_arn(self):

@patch("caching.base_tags_cache.send_forwarder_internal_metrics")
@patch("caching.lambda_cache.send_forwarder_internal_metrics")
@patch.dict(os.environ, {"DD_FETCH_LAMBDA_TAGS": "true"})
Copy link
Contributor

Choose a reason for hiding this comment

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

we can use something like

with patch.dict(os.environ, {"MY_ENV_VAR": "test_value"}):

to have local env changes, I think it would also allow not to reload settings module

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used here for example but it doesn't prevent the use of reload. If I remove it, the test is broken.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, at least the local env change part still holds if we're sharing / re-setting the same env vars amongst tests

def test_generate_enhanced_lambda_metrics_refresh_s3_cache(self, mock1, mock2):
reload(sys.modules["settings"])

tags_cache = LambdaTagsCache("")
tags_cache.get_cache_from_s3 = MagicMock(return_value=({}, 1000))
tags_cache.acquire_s3_cache_lock = MagicMock()
Expand Down Expand Up @@ -334,13 +339,12 @@ def test_generate_enhanced_lambda_metrics_refresh_s3_cache(self, mock1, mock2):
"timestamp": 10000,
}

os.environ["DD_FETCH_LAMBDA_TAGS"] = "True"
generate_enhanced_lambda_metrics(logs_input, tags_cache)
tags_cache.get_cache_from_s3.assert_called_once()
tags_cache.build_tags_cache.assert_called_once()
tags_cache.write_cache_to_s3.assert_called_once()
del os.environ["DD_FETCH_LAMBDA_TAGS"]

@patch.dict(os.environ, {"DD_FETCH_LAMBDA_TAGS": "true"})
@patch("caching.lambda_cache.LambdaTagsCache.release_s3_cache_lock")
@patch("caching.lambda_cache.LambdaTagsCache.acquire_s3_cache_lock")
@patch("caching.lambda_cache.LambdaTagsCache.get_resources_paginator")
Expand All @@ -358,6 +362,8 @@ def test_generate_enhanced_lambda_metrics_client_error(
mock_acquire_lock,
mock_release_lock,
):
reload(sys.modules["settings"])

mock_acquire_lock.return_value = True
mock_get_s3_cache.return_value = (
{},
Expand Down Expand Up @@ -391,22 +397,23 @@ def test_generate_enhanced_lambda_metrics_client_error(
"timestamp": 10000,
}

os.environ["DD_FETCH_LAMBDA_TAGS"] = "True"
generate_enhanced_lambda_metrics(logs_input, tags_cache)
mock_get_s3_cache.assert_called_once()
mock_get_s3_cache.reset_mock()
mock_get_resources_paginator.assert_called_once()
paginator.paginate.assert_called_once()
assert mock_base_tags_cache_forward_metrics.call_count == 1
assert mock_lambda_cache_forward_metrics.call_count == 2
del os.environ["DD_FETCH_LAMBDA_TAGS"]

@patch("caching.lambda_cache.send_forwarder_internal_metrics")
@patch("caching.base_tags_cache.send_forwarder_internal_metrics")
@patch("caching.lambda_cache.LambdaTagsCache.get_cache_from_s3")
@patch.dict(os.environ, {"DD_FETCH_LAMBDA_TAGS": "true"})
def test_generate_enhanced_lambda_metrics_timeout(
self, mock_get_s3_cache, mock_forward_metrics, mock_base_forward_metrics
):
reload(sys.modules["settings"])

mock_get_s3_cache.return_value = (
{
"arn:aws:lambda:us-east-1:0:function:cloudwatch-event": [
Expand Down Expand Up @@ -437,17 +444,18 @@ def test_generate_enhanced_lambda_metrics_timeout(
"timestamp": 1591714946151,
}

os.environ["DD_FETCH_LAMBDA_TAGS"] = "True"
generated_metrics = generate_enhanced_lambda_metrics(logs_input, tags_cache)
verify_as_json(generated_metrics)
del os.environ["DD_FETCH_LAMBDA_TAGS"]

@patch("caching.lambda_cache.send_forwarder_internal_metrics")
@patch("telemetry.send_forwarder_internal_metrics")
@patch("caching.lambda_cache.LambdaTagsCache.get_cache_from_s3")
@patch.dict(os.environ, {"DD_FETCH_LAMBDA_TAGS": "true"})
def test_generate_enhanced_lambda_metrics_out_of_memory(
self, mock_get_s3_cache, mock_forward_metrics, mock_base_forward_metrics
):
reload(sys.modules["settings"])

mock_get_s3_cache.return_value = (
{
"arn:aws:lambda:us-east-1:0:function:cloudwatch-event": [
Expand Down Expand Up @@ -478,10 +486,8 @@ def test_generate_enhanced_lambda_metrics_out_of_memory(
"timestamp": 1591714946151,
}

os.environ["DD_FETCH_LAMBDA_TAGS"] = "True"
generated_metrics = generate_enhanced_lambda_metrics(logs_input, tags_cache)
verify_as_json(generated_metrics)
del os.environ["DD_FETCH_LAMBDA_TAGS"]


if __name__ == "__main__":
Expand Down
23 changes: 22 additions & 1 deletion aws/logs_monitoring/tests/test_enrichment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import json
import os
import sys
import unittest
from unittest.mock import MagicMock
from importlib import reload
from unittest.mock import MagicMock, patch

from approvaltests.approvals import verify_as_json

Expand Down Expand Up @@ -181,7 +184,10 @@ def test_lambda_event_bad_arn(self):
add_metadata_to_lambda_log(event, cache_layer)
verify_as_json(event)

@patch.dict(os.environ, {"DD_FETCH_LAMBDA_TAGS": "false"})
def test_lambda_event_wo_service(self):
reload(sys.modules["settings"])

cache_layer = CacheLayer("")
event = {
"lambda": {
Expand All @@ -191,7 +197,10 @@ def test_lambda_event_wo_service(self):
add_metadata_to_lambda_log(event, cache_layer)
verify_as_json(event)

@patch.dict(os.environ, {"DD_FETCH_LAMBDA_TAGS": "true"})
def test_lambda_event_w_custom_tags_wo_service(self):
reload(sys.modules["settings"])

cache_layer = CacheLayer("")
cache_layer._lambda_cache.get = MagicMock(
return_value=["service:customtags_service"]
Expand All @@ -204,7 +213,10 @@ def test_lambda_event_w_custom_tags_wo_service(self):
add_metadata_to_lambda_log(event, cache_layer)
verify_as_json(event)

@patch.dict(os.environ, {"DD_FETCH_LAMBDA_TAGS": "true"})
def test_lambda_event_w_custom_tags_w_service(self):
reload(sys.modules["settings"])

cache_layer = CacheLayer("")
cache_layer._lambda_cache.get = MagicMock(
return_value=["service:customtags_service"]
Expand All @@ -218,7 +230,10 @@ def test_lambda_event_w_custom_tags_w_service(self):
add_metadata_to_lambda_log(event, cache_layer)
verify_as_json(event)

@patch.dict(os.environ, {"DD_FETCH_LAMBDA_TAGS": "false"})
def test_lambda_event_w_service(self):
reload(sys.modules["settings"])

cache_layer = CacheLayer("")
event = {
"lambda": {
Expand All @@ -229,7 +244,10 @@ def test_lambda_event_w_service(self):
add_metadata_to_lambda_log(event, cache_layer)
verify_as_json(event)

@patch.dict(os.environ, {"DD_FETCH_LAMBDA_TAGS": "false"})
def test_lambda_event_w_service_and_ddtags(self):
reload(sys.modules["settings"])

cache_layer = CacheLayer("")
event = {
"lambda": {
Expand All @@ -241,7 +259,10 @@ def test_lambda_event_w_service_and_ddtags(self):
add_metadata_to_lambda_log(event, cache_layer)
verify_as_json(event)

@patch.dict(os.environ, {"DD_FETCH_LAMBDA_TAGS": "true"})
def test_lambda_event_w_custom_tags_env(self):
reload(sys.modules["settings"])

cache_layer = CacheLayer("")
cache_layer._lambda_cache.get = MagicMock(return_value=["env:customtags_env"])
event = {
Expand Down
Loading