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: Add internal UUID types #616

Merged
merged 7 commits into from
Dec 10, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ build/sentry_test_unit: build
test: update-test-discovery test-integration
.PHONY: test

test-unit: update-test-discovery build/sentry_test_unit
./build/sentry_test_unit
Copy link
Contributor Author

Choose a reason for hiding this comment

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

on my machine this still spits out an absurd number of errors that look vaguely like

[71087:13339984:20211208,163001.841338:WARNING crash_report_database_mac.mm:767] Failed to read report metadata for .../sentry-native/.sentry-native/completed/49b5cb3a-64f0-4d5f-86fb-ed731f0981df.meta

i've tried negative grepping and awking this away to no luck; i would imagine in the worst case scenario this would require some sort of custom build that changes the logging level on crashpad to error at minimum. any help here would be much appreciated as the unit test results are unnecessarily difficult to parse through by default thanks to my poor shell skills.

Copy link
Member

Choose a reason for hiding this comment

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

You can ignore this. Just rm -rf .sentry-native, as that directory is safe to prune.
However now I think that we might want to have a solution for this, as I bet customers are hitting that as well sometimes and its not as simple to tell them "just remove that directory".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'll leave this as is for the time being. if a contributor ends up bumping into this problem we can loop back and look into addressing this then. the solutions i've explored so far to deal with this are all fairly ugly, so i'd rather take the hit and just manually prune the reports stored in my local DB.

.PHONY: test-unit

test-integration: setup-venv
.venv/bin/pytest tests --verbose
.PHONY: test-integration
Expand Down
4 changes: 2 additions & 2 deletions include/sentry.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ SENTRY_API sentry_value_t sentry_value_new_int32(int32_t value);
SENTRY_API sentry_value_t sentry_value_new_double(double value);

/**
* Creates a new boolen value.
* Creates a new boolean value.
*/
SENTRY_API sentry_value_t sentry_value_new_bool(int value);

Expand Down Expand Up @@ -582,7 +582,7 @@ SENTRY_API int sentry_envelope_write_to_file(
/**
* The Sentry Client Options.
*
* See https://docs.sentry.io/error-reporting/configuration/
* See https://docs.sentry.io/platforms/native/configuration/
*/
struct sentry_options_s;
typedef struct sentry_options_s sentry_options_t;
Expand Down
22 changes: 22 additions & 0 deletions src/sentry_uuid.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "sentry_boot.h"

#include "sentry_random.h"
#include "sentry_uuid.h"
#include <stdio.h>
#include <string.h>

Expand Down Expand Up @@ -101,6 +102,27 @@ sentry_uuid_as_string(const sentry_uuid_t *uuid, char str[37])
#undef B
}

void
internal_sentry_uuid_as_string(const sentry_uuid_t *uuid, char str[37])
{
#define B(X) (unsigned char)uuid->bytes[X]
snprintf(str, 37,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
snprintf(str, 37,
snprintf(str, 33,

Since you are only serializing 16 hex bytes, and a zero byte.

"%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%"
"02hhx%02hhx%02hhx%02hhx%02hhx%02hhx",
B(0), B(1), B(2), B(3), B(4), B(5), B(6), B(7), B(8), B(9), B(10),
B(11), B(12), B(13), B(14), B(15));
#undef B
}

void
sentry_span_uuid_as_string(const sentry_uuid_t *uuid, char str[17])
{
#define B(X) (unsigned char)uuid->bytes[X]
snprintf(str, 17, "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx", B(0),
B(1), B(2), B(3), B(4), B(5), B(6), B(7));
#undef B
}

#ifdef SENTRY_PLATFORM_WINDOWS
sentry_uuid_t
sentry__uuid_from_native(const GUID *guid)
Expand Down
11 changes: 11 additions & 0 deletions src/sentry_uuid.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@

#include "sentry_boot.h"

/**
* Converts a sentry UUID to a string representation used for internal
* sentry UUIDs such as event IDs.
*/
void internal_sentry_uuid_as_string(const sentry_uuid_t *uuid, char str[37]);

/**
* Converts a sentry UUID to a string representation used for span IDs.
*/
void sentry_span_uuid_as_string(const sentry_uuid_t *uuid, char str[17]);

#ifdef SENTRY_PLATFORM_WINDOWS
/**
* Create a new UUID from the windows-native GUID type.
Expand Down
25 changes: 25 additions & 0 deletions src/sentry_value.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "sentry_string.h"
#include "sentry_sync.h"
#include "sentry_utils.h"
#include "sentry_uuid.h"
#include "sentry_value.h"

/**
Expand Down Expand Up @@ -973,6 +974,30 @@ sentry__value_new_hexstring(const uint8_t *bytes, size_t len)
return sentry__value_new_string_owned(buf);
}

sentry_value_t
sentry__value_new_span_uuid(const sentry_uuid_t *uuid)
{
char *buf = sentry_malloc(17);
if (!buf) {
return sentry_value_new_null();
}
sentry_span_uuid_as_string(uuid, buf);
buf[16] = '\0';
return sentry__value_new_string_owned(buf);
}

sentry_value_t
sentry__value_new_internal_uuid(const sentry_uuid_t *uuid)
{
char *buf = sentry_malloc(37);
if (!buf) {
return sentry_value_new_null();
}
internal_sentry_uuid_as_string(uuid, buf);
buf[36] = '\0';
return sentry__value_new_string_owned(buf);
}

sentry_value_t
sentry__value_new_uuid(const sentry_uuid_t *uuid)
{
Expand Down
14 changes: 14 additions & 0 deletions src/sentry_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,20 @@ sentry_value_t sentry__value_new_addr(uint64_t addr);
*/
sentry_value_t sentry__value_new_hexstring(const uint8_t *bytes, size_t len);

/**
* Creates a new String Value from the `uuid` that conforms to
* the structure of a span ID.
* See also `sentry_span_uuid_as_string`.
*/
sentry_value_t sentry__value_new_span_uuid(const sentry_uuid_t *uuid);

/**
* Creates a new String Value from the `uuid` in a form meant for
* ingestion as an internal ID.
* See also `sentry_internal_uuid_as_string`.
*/
sentry_value_t sentry__value_new_internal_uuid(const sentry_uuid_t *uuid);

/**
* Creates a new String Value from the `uuid`.
* See also `sentry_uuid_as_string`.
Expand Down
22 changes: 21 additions & 1 deletion tests/unit/test_uuid.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "sentry_testsupport.h"
#include <sentry.h>
#include <sentry_uuid.h>

SENTRY_TEST(uuid_api)
{
Expand All @@ -25,4 +26,23 @@ SENTRY_TEST(uuid_v4)
sentry_uuid_as_bytes(&uuid, bytes);
TEST_CHECK(bytes[6] >> 4 == 4);
}
}
}

SENTRY_TEST(internal_uuid_api)
{
sentry_uuid_t uuid
= sentry_uuid_from_string("f391fdc0bb2743b18c0c183bc217d42b");
TEST_CHECK(!sentry_uuid_is_nil(&uuid));
char ibuf[37];
internal_sentry_uuid_as_string(&uuid, ibuf);
TEST_CHECK_STRING_EQUAL(ibuf, "f391fdc0bb2743b18c0c183bc217d42b");

char sbuf[17];
sentry_span_uuid_as_string(&uuid, sbuf);
TEST_CHECK_STRING_EQUAL(sbuf, "f391fdc0bb2743b1");

sentry_uuid_t span_id = sentry_uuid_from_string("f391fdc0bb2743b1");
TEST_CHECK(!sentry_uuid_is_nil(&span_id));
sentry_span_uuid_as_string(&span_id, sbuf);
TEST_CHECK_STRING_EQUAL(sbuf, "f391fdc0bb2743b1");
}
1 change: 1 addition & 0 deletions tests/unit/tests.inc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ XX(dsn_store_url_without_path)
XX(empty_transport)
XX(fuzz_json)
XX(init_failure)
XX(internal_uuid_api)
XX(invalid_dsn)
XX(invalid_proxy)
XX(iso_time)
Expand Down