Skip to content

CDRIVER-5638 Record both FaaS and container metadata when both are present #2016

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mdb-ad
Copy link

@mdb-ad mdb-ad commented May 18, 2025

Add environment variable checks for being in both a container and FaaS environment. Add that info to the handshake document. Sync spec test.

@mdb-ad mdb-ad marked this pull request as ready for review May 29, 2025 02:36
@mdb-ad mdb-ad requested a review from a team as a code owner May 29, 2025 02:36
@mdb-ad mdb-ad requested a review from kevinAlbs May 29, 2025 02:36
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

LGTM with a test suggestion and a question about the Windows path.

Comment on lines +660 to +664
bson_iter_t iter;
ASSERT (bson_iter_init (&iter, doc));
ASSERT (bson_iter_find_descendant (&iter, "container.orchestrator", &iter));
ASSERT (BSON_ITER_HOLDS_UTF8 (&iter));
ASSERT (strcmp ("kubernetes", bson_iter_utf8 (&iter, NULL)) == 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
bson_iter_t iter;
ASSERT (bson_iter_init (&iter, doc));
ASSERT (bson_iter_find_descendant (&iter, "container.orchestrator", &iter));
ASSERT (BSON_ITER_HOLDS_UTF8 (&iter));
ASSERT (strcmp ("kubernetes", bson_iter_utf8 (&iter, NULL)) == 0);
ASSERT_CMPSTR (bson_lookup_utf8 (doc, "container.orchestrator"), "kubernetes");
ASSERT_CMPSTR (bson_lookup_utf8 (doc, "env.name"), "aws.lambda");

Suggest simplifying with the test helper bson_lookup_utf8. Suggest checking env.name is set to aws.lambda (to check both aws and container fields are set).

Comment on lines +374 to +377
handshake->docker = false;
#ifdef _POSIX_VERSION
handshake->docker = (access ("/.dockerenv", F_OK) == 0);
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is a separate check needed if the Docker container is running Windows? Example:

#ifdef _WIN32
   handshake->docker = (_access_s ("C:\\.dockerenv", 0) == 0);
#else
   handshake->docker = (access ("/.dockerenv", F_OK) == 0);
#endif

On non-Windows, I expect access can be assumed to be available since POSIX features are enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants