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.

Sign up for GitHub

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

Make llvm::telemetry::Manager::preDispatch protected. #127114

Merged
merged 3 commits into from
Feb 15, 2025
Merged

Make llvm::telemetry::Manager::preDispatch protected. #127114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cabc961
make llvm::telemetry::Manager::preDispatch protected. It should not b…
oontvoo Feb 13, 2025
e1d87dc
added header and provide empty impl for preDispatch
oontvoo Feb 15, 2025
5606f6a
Merge branch 'main' into preDis
oontvoo Feb 15, 2025
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
9 changes: 5 additions & 4 deletions llvm/include/llvm/Telemetry/Telemetry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,6 @@ class Manager {
public:
virtual ~Manager() = default;

// Optional callback for subclasses to perform additional tasks before
// dispatching to Destinations.
virtual Error preDispatch(TelemetryInfo *Entry) = 0;

// Dispatch Telemetry data to the Destination(s).
// The argument is non-const because the Manager may add or remove
// data from the entry.
Expand All @@ -150,6 +146,11 @@ class Manager {
// Register a Destination.
void addDestination(std::unique_ptr<Destination> Destination);

protected:
// Optional callback for subclasses to perform additional tasks before
Copy link
Collaborator

Choose a reason for hiding this comment

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

If it's optional, maybe it should have a default implementation (doing nothing, successfully)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 this makes sense to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

done - added the empty impl for preDispatch

Copy link
Collaborator

Choose a reason for hiding this comment

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

It shouldn't really be empty, it should return Error::success() (that's what I tried to indicate with "doing nothing, successfully", sorry if I wasn't clear)

// dispatching to Destinations.
virtual Error preDispatch(TelemetryInfo *Entry) {}

private:
std::vector<std::unique_ptr<Destination>> Destinations;
};
Expand Down
13 changes: 13 additions & 0 deletions llvm/lib/Telemetry/Telemetry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
///
/// \file
/// This file provides the basic framework for Telemetry.
/// Refer to its documentation at llvm/docs/Telemetry.rst for more details.
//===---------------------------------------------------------------------===//

#include "llvm/Telemetry/Telemetry.h"

namespace llvm {
Expand Down