diff --git a/libstats/socket/include/stats_event.h b/libstats/socket/include/stats_event.h index ff842838b..35762989b 100644 --- a/libstats/socket/include/stats_event.h +++ b/libstats/socket/include/stats_event.h @@ -29,8 +29,9 @@ * AStatsEvent* event = AStatsEvent_obtain(); * * AStatsEvent_setAtomId(event, atomId); + * AStatsEvent_addBoolAnnotation(event, 5, false); // atom-level annotation * AStatsEvent_writeInt32(event, 24); - * AStatsEvent_addBoolAnnotation(event, 1, true); // annotations apply to the previous field + * AStatsEvent_addBoolAnnotation(event, 1, true); // annotation for preceding atom field * AStatsEvent_addInt32Annotation(event, 2, 128); * AStatsEvent_writeFloat(event, 2.0); * @@ -38,13 +39,8 @@ * AStatsEvent_write(event); * AStatsEvent_release(event); * - * Notes: - * (a) write_() and add__annotation() should be called in the order that fields - * and annotations are defined in the atom. - * (b) set_atom_id() can be called anytime before stats_event_write(). - * (c) add__annotation() calls apply to the previous field. - * (d) If errors occur, stats_event_write() will write a bitmask of the errors to the socket. - * (e) All strings should be encoded using UTF8. + * Note that calls to add atom fields and annotations should be made in the + * order that they are defined in the atom. */ #ifdef __cplusplus @@ -84,7 +80,7 @@ void AStatsEvent_build(AStatsEvent* event); int AStatsEvent_write(AStatsEvent* event); /** - * Frees the memory held by this StatsEvent + * Frees the memory held by this StatsEvent. * * After calling this, the StatsEvent must not be used or modified in any way. */ @@ -92,6 +88,8 @@ void AStatsEvent_release(AStatsEvent* event); /** * Sets the atom id for this StatsEvent. + * + * This function should be called immediately after AStatsEvent_obtain. **/ void AStatsEvent_setAtomId(AStatsEvent* event, uint32_t atomId); diff --git a/libstats/socket/stats_event.c b/libstats/socket/stats_event.c index b045d9341..24d2ea87e 100644 --- a/libstats/socket/stats_event.c +++ b/libstats/socket/stats_event.c @@ -29,7 +29,6 @@ #define POS_NUM_ELEMENTS 1 #define POS_TIMESTAMP (POS_NUM_ELEMENTS + sizeof(uint8_t)) #define POS_ATOM_ID (POS_TIMESTAMP + sizeof(uint8_t) + sizeof(uint64_t)) -#define POS_FIRST_FIELD (POS_ATOM_ID + sizeof(uint8_t) + sizeof(uint32_t)) /* LIMITS */ #define MAX_ANNOTATION_COUNT 15 @@ -66,8 +65,11 @@ // within a buf. Also includes other required fields. struct AStatsEvent { uint8_t* buf; - size_t lastFieldPos; // location of last field within the buf - size_t size; // number of valid bytes within buffer + // Location of last field within the buf. Here, field denotes either a + // metadata field (e.g. timestamp) or an atom field. + size_t lastFieldPos; + // Number of valid bytes within the buffer. + size_t size; uint32_t numElements; uint32_t atomId; uint32_t errors; @@ -85,20 +87,21 @@ static int64_t get_elapsed_realtime_ns() { AStatsEvent* AStatsEvent_obtain() { AStatsEvent* event = malloc(sizeof(AStatsEvent)); event->buf = (uint8_t*)calloc(MAX_EVENT_PAYLOAD, 1); - event->buf[0] = OBJECT_TYPE; + event->lastFieldPos = 0; + event->size = 2; // reserve first two bytes for outer event type and number of elements + event->numElements = 0; event->atomId = 0; event->errors = 0; event->truncate = true; // truncate for both pulled and pushed atoms event->built = false; - // place the timestamp - uint64_t timestampNs = get_elapsed_realtime_ns(); - event->buf[POS_TIMESTAMP] = INT64_TYPE; - memcpy(&event->buf[POS_TIMESTAMP + sizeof(uint8_t)], ×tampNs, sizeof(timestampNs)); + event->buf[0] = OBJECT_TYPE; + AStatsEvent_writeInt64(event, get_elapsed_realtime_ns()); // write the timestamp - event->numElements = 1; - event->lastFieldPos = 0; // 0 since we haven't written a field yet - event->size = POS_FIRST_FIELD; + // Force client to set atom id immediately (this is required for atom-level + // annotations to be written correctly). All atom field and annotation + // writes will fail until the atom id is set because event->errors != 0. + event->errors |= ERROR_NO_ATOM_ID; return event; } @@ -109,10 +112,12 @@ void AStatsEvent_release(AStatsEvent* event) { } void AStatsEvent_setAtomId(AStatsEvent* event, uint32_t atomId) { + if ((event->errors & ERROR_NO_ATOM_ID) == 0) return; + + // Clear the ERROR_NO_ATOM_ID bit. + event->errors &= ~ERROR_NO_ATOM_ID; event->atomId = atomId; - event->buf[POS_ATOM_ID] = INT32_TYPE; - memcpy(&event->buf[POS_ATOM_ID + sizeof(uint8_t)], &atomId, sizeof(atomId)); - event->numElements++; + AStatsEvent_writeInt32(event, atomId); } // Overwrites the timestamp populated in AStatsEvent_obtain with a custom @@ -306,23 +311,23 @@ void AStatsEvent_truncateBuffer(AStatsEvent* event, bool truncate) { void AStatsEvent_build(AStatsEvent* event) { if (event->built) return; - if (event->atomId == 0) event->errors |= ERROR_NO_ATOM_ID; - - if (event->numElements > MAX_BYTE_VALUE) { - event->errors |= ERROR_TOO_MANY_FIELDS; - } else { - event->buf[POS_NUM_ELEMENTS] = event->numElements; - } + if (event->numElements > MAX_BYTE_VALUE) event->errors |= ERROR_TOO_MANY_FIELDS; // If there are errors, rewrite buffer. if (event->errors) { - event->buf[POS_NUM_ELEMENTS] = 3; - event->buf[POS_FIRST_FIELD] = ERROR_TYPE; - memcpy(&event->buf[POS_FIRST_FIELD + sizeof(uint8_t)], &event->errors, - sizeof(event->errors)); - event->size = POS_FIRST_FIELD + sizeof(uint8_t) + sizeof(uint32_t); + // Discard everything after the atom id (including atom-level + // annotations). This leaves only two elements (timestamp and atom id). + event->numElements = 2; + // Reset number of atom-level annotations to 0. + event->buf[POS_ATOM_ID] = INT32_TYPE; + // Now, write errors to the buffer immediately after the atom id. + event->size = POS_ATOM_ID + sizeof(uint8_t) + sizeof(uint32_t); + start_field(event, ERROR_TYPE); + append_int32(event, event->errors); } + event->buf[POS_NUM_ELEMENTS] = event->numElements; + // Truncate the buffer to the appropriate length in order to limit our // memory usage. if (event->truncate) event->buf = (uint8_t*)realloc(event->buf, event->size); diff --git a/libstats/socket/tests/stats_event_test.cpp b/libstats/socket/tests/stats_event_test.cpp index 69d0a9b75..04eff3689 100644 --- a/libstats/socket/tests/stats_event_test.cpp +++ b/libstats/socket/tests/stats_event_test.cpp @@ -89,7 +89,7 @@ void checkAnnotation(uint8_t** buffer, uint8_t annotationId, uint8_t typeId, T a } void checkMetadata(uint8_t** buffer, uint8_t numElements, int64_t startTime, int64_t endTime, - uint32_t atomId) { + uint32_t atomId, uint8_t numAtomLevelAnnotations = 0) { // All events start with OBJECT_TYPE id. checkTypeHeader(buffer, OBJECT_TYPE); @@ -104,7 +104,7 @@ void checkMetadata(uint8_t** buffer, uint8_t numElements, int64_t startTime, int EXPECT_LE(timestamp, endTime); // Check atom id - checkTypeHeader(buffer, INT32_TYPE); + checkTypeHeader(buffer, INT32_TYPE, numAtomLevelAnnotations); checkScalar(buffer, atomId); } @@ -240,7 +240,7 @@ TEST(StatsEventTest, TestAttributionChains) { AStatsEvent_release(event); } -TEST(StatsEventTest, TestAnnotations) { +TEST(StatsEventTest, TestFieldAnnotations) { uint32_t atomId = 100; // first element information @@ -259,7 +259,7 @@ TEST(StatsEventTest, TestAnnotations) { int64_t startTime = android::elapsedRealtimeNano(); AStatsEvent* event = AStatsEvent_obtain(); - AStatsEvent_setAtomId(event, 100); + AStatsEvent_setAtomId(event, atomId); AStatsEvent_writeBool(event, boolValue); AStatsEvent_addBoolAnnotation(event, boolAnnotation1Id, boolAnnotation1Value); AStatsEvent_addInt32Annotation(event, boolAnnotation2Id, boolAnnotation2Value); @@ -292,6 +292,45 @@ TEST(StatsEventTest, TestAnnotations) { AStatsEvent_release(event); } +TEST(StatsEventTest, TestAtomLevelAnnotations) { + uint32_t atomId = 100; + // atom-level annotation information + uint8_t boolAnnotationId = 1; + uint8_t int32AnnotationId = 2; + bool boolAnnotationValue = false; + int32_t int32AnnotationValue = 5; + + float fieldValue = -3.5; + + int64_t startTime = android::elapsedRealtimeNano(); + AStatsEvent* event = AStatsEvent_obtain(); + AStatsEvent_setAtomId(event, atomId); + AStatsEvent_addBoolAnnotation(event, boolAnnotationId, boolAnnotationValue); + AStatsEvent_addInt32Annotation(event, int32AnnotationId, int32AnnotationValue); + AStatsEvent_writeFloat(event, fieldValue); + AStatsEvent_build(event); + int64_t endTime = android::elapsedRealtimeNano(); + + size_t bufferSize; + uint8_t* buffer = AStatsEvent_getBuffer(event, &bufferSize); + uint8_t* bufferEnd = buffer + bufferSize; + + checkMetadata(&buffer, /*numElements=*/1, startTime, endTime, atomId, + /*numAtomLevelAnnotations=*/2); + + // check atom-level annotations + checkAnnotation(&buffer, boolAnnotationId, BOOL_TYPE, boolAnnotationValue); + checkAnnotation(&buffer, int32AnnotationId, INT32_TYPE, int32AnnotationValue); + + // check first element + checkTypeHeader(&buffer, FLOAT_TYPE); + checkScalar(&buffer, fieldValue); + + EXPECT_EQ(buffer, bufferEnd); // ensure that we have read the entire buffer + EXPECT_EQ(AStatsEvent_getErrors(event), 0); + AStatsEvent_release(event); +} + TEST(StatsEventTest, TestNoAtomIdError) { AStatsEvent* event = AStatsEvent_obtain(); // Don't set the atom id in order to trigger the error.