From 83aaa66b469115cfb4e9db39e3534a271812019c Mon Sep 17 00:00:00 2001 From: Muhammad Qureshi Date: Tue, 21 Apr 2020 12:51:36 -0700 Subject: [PATCH] Add LogEvent::hasAttributionChain() Optionally, accept a std::pair* that is populated with the starting and ending indices of attribution nodes in FieldValues vector. Bug: 154286011 Test: bit statsd_test:* Change-Id: I6f19abea7fbb27712b6c2d5a7679aa4f428c191d --- cmds/statsd/src/FieldValue.h | 1 - cmds/statsd/src/StatsLogProcessor.cpp | 15 +++++++-------- cmds/statsd/src/external/puller_util.cpp | 18 +++++++++--------- cmds/statsd/src/logd/LogEvent.cpp | 23 ++++++++++++++++++++--- cmds/statsd/src/logd/LogEvent.h | 13 ++++++------- cmds/statsd/tests/LogEvent_test.cpp | 8 ++++++++ 6 files changed, 50 insertions(+), 28 deletions(-) diff --git a/cmds/statsd/src/FieldValue.h b/cmds/statsd/src/FieldValue.h index e251399776fb6..afb55f0693e10 100644 --- a/cmds/statsd/src/FieldValue.h +++ b/cmds/statsd/src/FieldValue.h @@ -27,7 +27,6 @@ struct Matcher; struct Field; struct FieldValue; -const int32_t kAttributionField = 1; const int32_t kMaxLogDepth = 2; const int32_t kLastBitMask = 0x80; const int32_t kClearLastBitDeco = 0x7f; diff --git a/cmds/statsd/src/StatsLogProcessor.cpp b/cmds/statsd/src/StatsLogProcessor.cpp index d914ab2436c7c..50fe47dfed03b 100644 --- a/cmds/statsd/src/StatsLogProcessor.cpp +++ b/cmds/statsd/src/StatsLogProcessor.cpp @@ -139,14 +139,13 @@ void StatsLogProcessor::onPeriodicAlarmFired( } void StatsLogProcessor::mapIsolatedUidToHostUidIfNecessaryLocked(LogEvent* event) const { - if (event->getAttributionChainIndex() != -1) { - for (auto& value : *(event->getMutableValues())) { - if (value.mField.getPosAtDepth(0) > kAttributionField) { - break; - } - if (isAttributionUidField(value)) { - const int hostUid = mUidMap->getHostUidOrSelf(value.mValue.int_value); - value.mValue.setInt(hostUid); + if (std::pair indexRange; event->hasAttributionChain(&indexRange)) { + vector* const fieldValues = event->getMutableValues(); + for (int i = indexRange.first; i <= indexRange.second; i++) { + FieldValue& fieldValue = fieldValues->at(i); + if (isAttributionUidField(fieldValue)) { + const int hostUid = mUidMap->getHostUidOrSelf(fieldValue.mValue.int_value); + fieldValue.mValue.setInt(hostUid); } } } else { diff --git a/cmds/statsd/src/external/puller_util.cpp b/cmds/statsd/src/external/puller_util.cpp index 9e72a23d27f43..c688133f9bb67 100644 --- a/cmds/statsd/src/external/puller_util.cpp +++ b/cmds/statsd/src/external/puller_util.cpp @@ -51,7 +51,8 @@ void mapAndMergeIsolatedUidsToHostUid(vector>& data, const int tagId, const vector& additiveFieldsVec) { // Check the first LogEvent for attribution chain or a uid field as either all atoms with this // tagId have them or none of them do. - const bool hasAttributionChain = data[0]->getAttributionChainIndex() != -1; + std::pair attrIndexRange; + const bool hasAttributionChain = data[0]->hasAttributionChain(&attrIndexRange); bool hasUidField = (data[0]->getUidFieldIndex() != -1); if (!hasAttributionChain && !hasUidField) { @@ -65,14 +66,13 @@ void mapAndMergeIsolatedUidsToHostUid(vector>& data, const ALOGE("Wrong atom. Expecting %d, got %d", tagId, event->GetTagId()); return; } - if (event->getAttributionChainIndex() != -1) { - for (auto& value : *(event->getMutableValues())) { - if (value.mField.getPosAtDepth(0) > kAttributionField) { - break; - } - if (isAttributionUidField(value)) { - const int hostUid = uidMap->getHostUidOrSelf(value.mValue.int_value); - value.mValue.setInt(hostUid); + if (hasAttributionChain) { + vector* const fieldValues = event->getMutableValues(); + for (int i = attrIndexRange.first; i <= attrIndexRange.second; i++) { + FieldValue& fieldValue = fieldValues->at(i); + if (isAttributionUidField(fieldValue)) { + const int hostUid = uidMap->getHostUidOrSelf(fieldValue.mValue.int_value); + fieldValue.mValue.setInt(hostUid); } } } else { diff --git a/cmds/statsd/src/logd/LogEvent.cpp b/cmds/statsd/src/logd/LogEvent.cpp index 61cd01728ab1d..18f1b825cb5fa 100644 --- a/cmds/statsd/src/logd/LogEvent.cpp +++ b/cmds/statsd/src/logd/LogEvent.cpp @@ -219,8 +219,8 @@ void LogEvent::parseKeyValuePairs(int32_t* pos, int32_t depth, bool* last, uint8 void LogEvent::parseAttributionChain(int32_t* pos, int32_t depth, bool* last, uint8_t numAnnotations) { - int firstUidInChainIndex = mValues.size(); - int32_t numNodes = readNextValue(); + const unsigned int firstUidInChainIndex = mValues.size(); + const int32_t numNodes = readNextValue(); for (pos[1] = 1; pos[1] <= numNodes; pos[1]++) { last[1] = (pos[1] == numNodes); @@ -233,6 +233,11 @@ void LogEvent::parseAttributionChain(int32_t* pos, int32_t depth, bool* last, last[2] = true; parseString(pos, /*depth=*/2, last, /*numAnnotations=*/0); } + // Check if at least one node was successfully parsed. + if (mValues.size() - 1 > firstUidInChainIndex) { + mAttributionChainStartIndex = firstUidInChainIndex; + mAttributionChainEndIndex = mValues.size() - 1; + } parseAnnotations(numAnnotations, firstUidInChainIndex); @@ -411,7 +416,6 @@ bool LogEvent::parseBuffer(uint8_t* buf, size_t len) { break; case ATTRIBUTION_CHAIN_TYPE: parseAttributionChain(pos, /*depth=*/0, last, getNumAnnotations(typeInfo)); - if (mAttributionChainIndex == -1) mAttributionChainIndex = pos[0]; break; default: mValid = false; @@ -572,6 +576,19 @@ void LogEvent::ToProto(ProtoOutputStream& protoOutput) const { writeFieldValueTreeToStream(mTagId, getValues(), &protoOutput); } +bool LogEvent::hasAttributionChain(std::pair* indexRange) const { + if (mAttributionChainStartIndex == -1 || mAttributionChainEndIndex == -1) { + return false; + } + + if (nullptr != indexRange) { + indexRange->first = mAttributionChainStartIndex; + indexRange->second = mAttributionChainEndIndex; + } + + return true; +} + void writeExperimentIdsToProto(const std::vector& experimentIds, std::vector* protoOut) { ProtoOutputStream proto; diff --git a/cmds/statsd/src/logd/LogEvent.h b/cmds/statsd/src/logd/LogEvent.h index 41fdcc2cbe7a6..e7340ef2fa06d 100644 --- a/cmds/statsd/src/logd/LogEvent.h +++ b/cmds/statsd/src/logd/LogEvent.h @@ -163,12 +163,10 @@ public: return mUidFieldIndex; } - // Returns the index of (the first) attribution chain within the atom - // definition. Note that the value is 1-indexed. If there is no attribution - // chain, returns -1. - inline int getAttributionChainIndex() { - return mAttributionChainIndex; - } + // Returns whether this LogEvent has an AttributionChain. + // If it does and indexRange is not a nullptr, populate indexRange with the start and end index + // of the AttributionChain within mValues. + bool hasAttributionChain(std::pair* indexRange = nullptr) const; // Returns the index of the exclusive state field within the FieldValues vector if // an exclusive state exists. If there is no exclusive state field, returns -1. @@ -310,7 +308,8 @@ private: // Annotations bool mTruncateTimestamp = false; int mUidFieldIndex = -1; - int mAttributionChainIndex = -1; + int mAttributionChainStartIndex = -1; + int mAttributionChainEndIndex = -1; int mExclusiveStateFieldIndex = -1; }; diff --git a/cmds/statsd/tests/LogEvent_test.cpp b/cmds/statsd/tests/LogEvent_test.cpp index bb4578d9b701e..e2c2743e1bb50 100644 --- a/cmds/statsd/tests/LogEvent_test.cpp +++ b/cmds/statsd/tests/LogEvent_test.cpp @@ -95,6 +95,7 @@ TEST(LogEventTest, TestPrimitiveParsing) { EXPECT_EQ(100, logEvent.GetTagId()); EXPECT_EQ(1000, logEvent.GetUid()); EXPECT_EQ(1001, logEvent.GetPid()); + EXPECT_FALSE(logEvent.hasAttributionChain()); const vector& values = logEvent.getValues(); EXPECT_EQ(4, values.size()); @@ -143,6 +144,7 @@ TEST(LogEventTest, TestStringAndByteArrayParsing) { EXPECT_EQ(100, logEvent.GetTagId()); EXPECT_EQ(1000, logEvent.GetUid()); EXPECT_EQ(1001, logEvent.GetPid()); + EXPECT_FALSE(logEvent.hasAttributionChain()); const vector& values = logEvent.getValues(); EXPECT_EQ(2, values.size()); @@ -179,6 +181,7 @@ TEST(LogEventTest, TestEmptyString) { EXPECT_EQ(100, logEvent.GetTagId()); EXPECT_EQ(1000, logEvent.GetUid()); EXPECT_EQ(1001, logEvent.GetPid()); + EXPECT_FALSE(logEvent.hasAttributionChain()); const vector& values = logEvent.getValues(); EXPECT_EQ(1, values.size()); @@ -248,6 +251,11 @@ TEST(LogEventTest, TestAttributionChain) { const vector& values = logEvent.getValues(); EXPECT_EQ(4, values.size()); // 2 per attribution node + std::pair attrIndexRange; + EXPECT_TRUE(logEvent.hasAttributionChain(&attrIndexRange)); + EXPECT_EQ(0, attrIndexRange.first); + EXPECT_EQ(3, attrIndexRange.second); + // Check first attribution node const FieldValue& uid1Item = values[0]; Field expectedField = getField(100, {1, 1, 1}, 2, {true, false, false});