Trace Redaction - Consolidate Filters
There were two filter types checking the same things. This replaces
them with a more general filter interface. This should make it
easier to implement filters correctly.
Change-Id: I6c6488428d6c2134ee668077bf1b9302da220044
diff --git a/Android.bp b/Android.bp
index dbac421..2b9993f 100644
--- a/Android.bp
+++ b/Android.bp
@@ -13608,6 +13608,7 @@
"src/trace_redaction/collect_system_info.cc",
"src/trace_redaction/collect_timeline_events.cc",
"src/trace_redaction/filter_packet_using_allowlist.cc",
+ "src/trace_redaction/filtering.cc",
"src/trace_redaction/find_package_uid.cc",
"src/trace_redaction/modify_process_trees.cc",
"src/trace_redaction/populate_allow_lists.cc",
diff --git a/src/trace_redaction/BUILD.gn b/src/trace_redaction/BUILD.gn
index b6532cb..03fbe6a 100644
--- a/src/trace_redaction/BUILD.gn
+++ b/src/trace_redaction/BUILD.gn
@@ -36,6 +36,8 @@
"collect_timeline_events.h",
"filter_packet_using_allowlist.cc",
"filter_packet_using_allowlist.h",
+ "filtering.cc",
+ "filtering.h",
"find_package_uid.cc",
"find_package_uid.h",
"frame_cookie.h",
diff --git a/src/trace_redaction/filtering.cc b/src/trace_redaction/filtering.cc
new file mode 100644
index 0000000..2666592
--- /dev/null
+++ b/src/trace_redaction/filtering.cc
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/trace_redaction/filtering.h"
+
+namespace perfetto::trace_redaction {
+
+PidFilter::~PidFilter() = default;
+
+bool ConnectedToPackage::Includes(const Context& context,
+ uint64_t ts,
+ int32_t pid) const {
+ PERFETTO_DCHECK(context.timeline);
+ PERFETTO_DCHECK(context.package_uid.has_value());
+ return context.timeline->PidConnectsToUid(ts, pid, *context.package_uid);
+}
+
+bool AllowAll::Includes(const Context&, uint64_t, int32_t) const {
+ return true;
+}
+
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/filtering.h b/src/trace_redaction/filtering.h
new file mode 100644
index 0000000..488057d
--- /dev/null
+++ b/src/trace_redaction/filtering.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <cstdint>
+
+#include "src/trace_redaction/trace_redaction_framework.h"
+
+#ifndef SRC_TRACE_REDACTION_FILTERING_H_
+#define SRC_TRACE_REDACTION_FILTERING_H_
+
+namespace perfetto::trace_redaction {
+
+class PidFilter {
+ public:
+ virtual ~PidFilter();
+
+ virtual bool Includes(const Context& context,
+ uint64_t ts,
+ int32_t pid) const = 0;
+};
+
+class ConnectedToPackage : public PidFilter {
+ public:
+ bool Includes(const Context& context,
+ uint64_t ts,
+ int32_t pid) const override;
+};
+
+class AllowAll : public PidFilter {
+ public:
+ bool Includes(const Context&, uint64_t, int32_t) const override;
+};
+
+} // namespace perfetto::trace_redaction
+
+#endif // SRC_TRACE_REDACTION_FILTERING_H_
diff --git a/src/trace_redaction/main.cc b/src/trace_redaction/main.cc
index a60b7f4..eb08826 100644
--- a/src/trace_redaction/main.cc
+++ b/src/trace_redaction/main.cc
@@ -99,7 +99,7 @@
{
auto* primitive = redactor.emplace_transform<RedactProcessTrees>();
primitive->emplace_modifier<ProcessTreeCreateSynthThreads>();
- primitive->emplace_filter<ProcessTreeFilterConnectedToPackage>();
+ primitive->emplace_filter<ConnectedToPackage>();
}
Context context;
diff --git a/src/trace_redaction/redact_process_events.h b/src/trace_redaction/redact_process_events.h
index 4ada317..905d66a 100644
--- a/src/trace_redaction/redact_process_events.h
+++ b/src/trace_redaction/redact_process_events.h
@@ -100,7 +100,7 @@
std::unique_ptr<SchedEventModifier> modifier_;
- std::unique_ptr<SchedEventFilter> filter_;
+ std::unique_ptr<PidFilter> filter_;
};
} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/redact_process_trees.cc b/src/trace_redaction/redact_process_trees.cc
index c72cdc6..37dc933 100644
--- a/src/trace_redaction/redact_process_trees.cc
+++ b/src/trace_redaction/redact_process_trees.cc
@@ -30,27 +30,8 @@
namespace perfetto::trace_redaction {
-ProcessTreeFilter::~ProcessTreeFilter() = default;
-
ProcessTreeModifier::~ProcessTreeModifier() = default;
-bool ProcessTreeFilterAllowAll::Filter(const Context&,
- uint64_t,
- int32_t) const {
- return true;
-}
-
-bool ProcessTreeFilterConnectedToPackage::Filter(const Context& context,
- uint64_t ts,
- int32_t pid) const {
- PERFETTO_DCHECK(context.timeline);
- PERFETTO_DCHECK(context.package_uid.has_value());
-
- // Checking the uid directly is an option, but the timeline handles multiple
- // instances of the same app.
- return context.timeline->PidConnectsToUid(ts, pid, *context.package_uid);
-}
-
base::Status ProcessTreeDoNothing::Modify(const Context&,
protos::pbzero::ProcessTree*) const {
return base::OkStatus();
@@ -176,7 +157,7 @@
PERFETTO_DCHECK(filter_);
- if (filter_->Filter(context, ts, pid.as_int32())) {
+ if (filter_->Includes(context, ts, pid.as_int32())) {
proto_util::AppendField(field, message);
}
@@ -198,7 +179,7 @@
PERFETTO_DCHECK(filter_);
- if (filter_->Filter(context, ts, tid.as_int32())) {
+ if (filter_->Includes(context, ts, tid.as_int32())) {
proto_util::AppendField(field, message);
}
diff --git a/src/trace_redaction/redact_process_trees.h b/src/trace_redaction/redact_process_trees.h
index 4bbd32f..5c9e507 100644
--- a/src/trace_redaction/redact_process_trees.h
+++ b/src/trace_redaction/redact_process_trees.h
@@ -19,20 +19,13 @@
#include "perfetto/base/status.h"
#include "perfetto/protozero/field.h"
+#include "src/trace_redaction/redact_sched_events.h"
#include "src/trace_redaction/trace_redaction_framework.h"
#include "protos/perfetto/trace/ps/process_tree.pbzero.h"
namespace perfetto::trace_redaction {
-class ProcessTreeFilter {
- public:
- virtual ~ProcessTreeFilter();
- virtual bool Filter(const Context& context,
- uint64_t ts,
- int32_t pid) const = 0;
-};
-
class ProcessTreeModifier {
public:
virtual ~ProcessTreeModifier();
@@ -40,16 +33,6 @@
protos::pbzero::ProcessTree* message) const = 0;
};
-class ProcessTreeFilterAllowAll : public ProcessTreeFilter {
- public:
- bool Filter(const Context& context, uint64_t ts, int32_t pid) const override;
-};
-
-class ProcessTreeFilterConnectedToPackage : public ProcessTreeFilter {
- public:
- bool Filter(const Context& context, uint64_t ts, int32_t pid) const override;
-};
-
class ProcessTreeDoNothing : public ProcessTreeModifier {
public:
base::Status Modify(const Context& context,
@@ -98,7 +81,7 @@
base::Status AppendSynthThreads(const Context& context,
protos::pbzero::ProcessTree* message) const;
- std::unique_ptr<ProcessTreeFilter> filter_;
+ std::unique_ptr<PidFilter> filter_;
std::unique_ptr<ProcessTreeModifier> modifier_;
};
diff --git a/src/trace_redaction/redact_process_trees_integrationtest.cc b/src/trace_redaction/redact_process_trees_integrationtest.cc
index 2b517d0..44ec1ea 100644
--- a/src/trace_redaction/redact_process_trees_integrationtest.cc
+++ b/src/trace_redaction/redact_process_trees_integrationtest.cc
@@ -56,7 +56,7 @@
auto* process_tree =
trace_redactor()->emplace_transform<RedactProcessTrees>();
process_tree->emplace_modifier<ProcessTreeDoNothing>();
- process_tree->emplace_filter<ProcessTreeFilterConnectedToPackage>();
+ process_tree->emplace_filter<ConnectedToPackage>();
// In this case, the process and package have the same name.
context()->package_name = kProcessName;
@@ -186,7 +186,7 @@
auto* process_tree =
trace_redactor()->emplace_transform<RedactProcessTrees>();
process_tree->emplace_modifier<ProcessTreeCreateSynthThreads>();
- process_tree->emplace_filter<ProcessTreeFilterAllowAll>();
+ process_tree->emplace_filter<AllowAll>();
ASSERT_OK(Redact());
@@ -209,7 +209,7 @@
auto* process_tree =
trace_redactor()->emplace_transform<RedactProcessTrees>();
process_tree->emplace_modifier<ProcessTreeCreateSynthThreads>();
- process_tree->emplace_filter<ProcessTreeFilterAllowAll>();
+ process_tree->emplace_filter<AllowAll>();
ASSERT_OK(Redact());
diff --git a/src/trace_redaction/redact_sched_events.cc b/src/trace_redaction/redact_sched_events.cc
index 9452545..87f87a5 100644
--- a/src/trace_redaction/redact_sched_events.cc
+++ b/src/trace_redaction/redact_sched_events.cc
@@ -108,8 +108,6 @@
SchedEventModifier::~SchedEventModifier() = default;
-SchedEventFilter::~SchedEventFilter() = default;
-
base::Status RedactSchedEvents::Transform(const Context& context,
std::string* packet) const {
PERFETTO_DCHECK(modifier_);
@@ -652,16 +650,4 @@
return base::OkStatus();
}
-bool ConnectedToPackage::Includes(const Context& context,
- uint64_t ts,
- int32_t wakee) const {
- PERFETTO_DCHECK(context.package_uid.has_value());
-
- return context.timeline->PidConnectsToUid(ts, wakee, *context.package_uid);
-}
-
-bool AllowAll::Includes(const Context&, uint64_t, int32_t) const {
- return true;
-}
-
} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/redact_sched_events.h b/src/trace_redaction/redact_sched_events.h
index d035fdd..47ff373 100644
--- a/src/trace_redaction/redact_sched_events.h
+++ b/src/trace_redaction/redact_sched_events.h
@@ -17,6 +17,7 @@
#ifndef SRC_TRACE_REDACTION_REDACT_SCHED_EVENTS_H_
#define SRC_TRACE_REDACTION_REDACT_SCHED_EVENTS_H_
+#include "src/trace_redaction/filtering.h"
#include "src/trace_redaction/trace_redaction_framework.h"
#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
@@ -54,19 +55,6 @@
std::string* comm) const = 0;
};
-class SchedEventFilter {
- public:
- virtual ~SchedEventFilter();
-
- // The filter only exposes the wakee, and not the waker, because most
- // filtering logic only needs the wakee and while handling the waker logic
- // for ftrace events is trival, handling it for compact sched is non-trival
- // and easily implemented wrong.
- virtual bool Includes(const Context& context,
- uint64_t ts,
- int32_t target) const = 0;
-};
-
class RedactSchedEvents : public TransformPrimitive {
public:
base::Status Transform(const Context& context,
@@ -134,7 +122,7 @@
const;
std::unique_ptr<SchedEventModifier> modifier_;
- std::unique_ptr<SchedEventFilter> filter_;
+ std::unique_ptr<PidFilter> filter_;
};
class ClearComms : public SchedEventModifier {
@@ -155,18 +143,6 @@
std::string* comm) const override;
};
-class ConnectedToPackage : public SchedEventFilter {
- public:
- bool Includes(const Context& context,
- uint64_t ts,
- int32_t wakee) const override;
-};
-
-class AllowAll : public SchedEventFilter {
- public:
- bool Includes(const Context&, uint64_t, int32_t) const override;
-};
-
} // namespace perfetto::trace_redaction
#endif // SRC_TRACE_REDACTION_REDACT_SCHED_EVENTS_H_
