-
Notifications
You must be signed in to change notification settings - Fork 454
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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.
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
handshake->docker = false; | ||
#ifdef _POSIX_VERSION | ||
handshake->docker = (access ("/.dockerenv", F_OK) == 0); | ||
#endif |
There was a problem hiding this comment.
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.
Add environment variable checks for being in both a container and FaaS environment. Add that info to the handshake document. Sync spec test.