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

net: Add bytes sent/received metrics #2629

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+97 −2
Open

net: Add bytes sent/received metrics #2629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions include/seastar/net/api.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,19 @@ public:
bool supports_ipv6() const;
};

struct statistics {
uint64_t bytes_sent = 0;
uint64_t bytes_received = 0;
};
Copy link
Member

Choose a reason for hiding this comment

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

Minor, but such pure data classes are better as structs with exposed data members and inline initializers for all data members. Less noisy.

Copy link
Contributor

@travisdowns travisdowns Jan 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We did investigate the struct option on our end, but it added 1 instruction to the hot path and as we knew Avi-instruction-counter would be reviewing this we went with the separateed approach.

The extra instruction is probably only on x86 because at 16 bytes the struct can't be indexed with indexed addressing which maxes out at * 8, so it needs a separate shl do to the indexing.

Copy link
Member

Choose a reason for hiding this comment

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

How can struct/class possibly matter here? The layout is the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry @avikivity I didn't look carefully enough, I though you were referring to the separate read/write TLS variables, which are not bundled into a struct, not this one which is used to expose the metrics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed


namespace metrics {
class metric_groups;
class label_instance;
}

void register_net_metrics_for_scheduling_group(
metrics::metric_groups& m, unsigned sg_id, const metrics::label_instance& name);

class network_stack {
public:
virtual ~network_stack() {}
Expand All @@ -468,6 +481,11 @@ public:
return false;
}

// Return network stats (bytes sent/received etc.) for this stack and scheduling group
virtual statistics stats(unsigned scheduling_group_id) = 0;
// Clears the stats for this stack and scheduling group
virtual void clear_stats(unsigned scheduling_group_id) = 0;

/**
* Returns available network interfaces. This represents a
* snapshot of interfaces available at call time, hence the
Expand Down
2 changes: 2 additions & 0 deletions include/seastar/net/posix-stack.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ public:
virtual bool has_per_core_namespace() override { return _reuseport; };
bool supports_ipv6() const override;
std::vector<network_interface> network_interfaces() override;
virtual statistics stats(unsigned scheduling_group_id) override;
virtual void clear_stats(unsigned scheduling_group_id) override;
};

class posix_ap_network_stack : public posix_network_stack {
Expand Down
4 changes: 3 additions & 1 deletion src/core/reactor.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,9 @@ reactor::task_queue::register_stats() {
}, sm::description("Total amount in milliseconds we were in violation of the task quota"),
{group_label}),
});

register_net_metrics_for_scheduling_group(new_metrics, _id, group_label);

Copy link
Member

Choose a reason for hiding this comment

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

This is the only call, so new groups won't have metrics.

Copy link
Member

Choose a reason for hiding this comment

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

Wait that's completely wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I don't follow. This gets called whenever a new group is added (or renamed). It works effectively the same way as scheduling groups get their metrics right now.

Copy link
Member

Choose a reason for hiding this comment

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

"that's completely wrong" referred to my previous comment

_metrics = std::exchange(new_metrics, {});
}

Expand Down Expand Up @@ -2560,7 +2563,6 @@ void reactor::register_metrics() {
sm::make_counter("abandoned_failed_futures", _abandoned_failed_futures, sm::description("Total number of abandoned failed futures, futures destroyed while still containing an exception")),
});

namespace sm = seastar::metrics;
_metric_groups.add_group("reactor", {
sm::make_counter("fstream_reads", _io_stats.fstream_reads,
sm::description(
Expand Down
15 changes: 15 additions & 0 deletions src/net/native-stack-impl.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,17 @@ namespace seastar {

extern logger seastar_logger;

namespace internal {

namespace native_stack_net_stats {

inline thread_local std::array<uint64_t, max_scheduling_groups()> bytes_sent = {};
inline thread_local std::array<uint64_t, max_scheduling_groups()> bytes_received = {};

};

}

namespace net {

using namespace seastar;
Expand Down Expand Up @@ -172,6 +183,8 @@ public:
}
return _conn->wait_for_data().then([this] {
_buf = _conn->read();
auto sg_id = internal::scheduling_group_index(current_scheduling_group());
internal::native_stack_net_stats::bytes_received[sg_id] += _buf.len();
_cur_frag = 0;
_eof = !_buf.len();
return get();
Expand All @@ -193,6 +206,8 @@ public:
: _conn(std::move(conn)) {}
using data_sink_impl::put;
virtual future<> put(packet p) override {
auto sg_id = internal::scheduling_group_index(current_scheduling_group());
internal::native_stack_net_stats::bytes_sent[sg_id] += p.len();
return _conn->send(std::move(p));
}
virtual future<> close() override {
Expand Down
14 changes: 13 additions & 1 deletion src/net/native-stack.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,18 @@ class native_network_stack : public network_stack {
friend class native_network_interface;

std::vector<network_interface> network_interfaces() override;

virtual statistics stats(unsigned scheduling_group_id) override {
return statistics{
internal::native_stack_net_stats::bytes_sent[scheduling_group_id],
internal::native_stack_net_stats::bytes_received[scheduling_group_id],
};
}

virtual void clear_stats(unsigned scheduling_group_id) override {
internal::native_stack_net_stats::bytes_sent[scheduling_group_id] = 0;
internal::native_stack_net_stats::bytes_received[scheduling_group_id] = 0;
}
};

thread_local promise<std::unique_ptr<network_stack>> native_network_stack::ready_promise;
Expand Down Expand Up @@ -427,6 +439,6 @@ std::vector<network_interface> native_network_stack::network_interfaces() {
return res;
}

}
} // namespace net

}
27 changes: 27 additions & 0 deletions src/net/posix-stack.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ copy_reinterpret_cast(const void* ptr) {
return tmp;
}

thread_local std::array<uint64_t, seastar::max_scheduling_groups()> bytes_sent = {};
thread_local std::array<uint64_t, seastar::max_scheduling_groups()> bytes_received = {};

}

namespace seastar {
Expand Down Expand Up @@ -637,6 +640,8 @@ posix_data_source_impl::get() {
_config.buffer_size /= 2;
_config.buffer_size = std::max(_config.buffer_size, _config.min_buffer_size);
}
auto sg_id = internal::scheduling_group_index(current_scheduling_group());
bytes_received[sg_id] += b.size();
return b;
});
}
Expand Down Expand Up @@ -671,12 +676,16 @@ std::vector<iovec> to_iovec(std::vector<temporary_buffer<char>>& buf_vec) {

future<>
posix_data_sink_impl::put(temporary_buffer<char> buf) {
auto sg_id = internal::scheduling_group_index(current_scheduling_group());
bytes_sent[sg_id] += buf.size();
return _fd.write_all(buf.get(), buf.size()).then([d = buf.release()] {});
}

future<>
posix_data_sink_impl::put(packet p) {
_p = std::move(p);
auto sg_id = internal::scheduling_group_index(current_scheduling_group());
bytes_sent[sg_id] += _p.len();
return _fd.write_all(_p).then([this] { _p.reset(); });
}

Expand Down Expand Up @@ -876,13 +885,17 @@ future<> posix_datagram_channel::send(const socket_address& dst, const char *mes
auto len = strlen(message);
auto a = dst;
resolve_outgoing_address(a);
auto sg_id = internal::scheduling_group_index(current_scheduling_group());
bytes_sent[sg_id] += len;
return _fd.sendto(a, message, len)
.then([len] (size_t size) { assert(size == len); });
}

future<> posix_datagram_channel::send(const socket_address& dst, packet p) {
auto len = p.len();
_send.prepare(dst, std::move(p));
auto sg_id = internal::scheduling_group_index(current_scheduling_group());
bytes_sent[sg_id] += len;
return _fd.sendmsg(&_send._hdr)
.then([len] (size_t size) { assert(size == len); });
}
Expand Down Expand Up @@ -954,6 +967,8 @@ posix_datagram_channel::receive() {
break;
}
}
auto sg_id = internal::scheduling_group_index(current_scheduling_group());
bytes_received[sg_id] += size;
return make_ready_future<datagram>(datagram(std::make_unique<posix_datagram>(
_recv._src_addr, dst ? *dst : _address, packet(fragment{_recv._buffer, size}, make_deleter([buf = _recv._buffer] { delete[] buf; })))));
}).handle_exception([p = _recv._buffer](auto ep) {
Expand Down Expand Up @@ -1199,6 +1214,18 @@ std::vector<network_interface> posix_network_stack::network_interfaces() {
return std::vector<network_interface>(thread_local_interfaces.begin(), thread_local_interfaces.end());
}

statistics posix_network_stack::stats(unsigned scheduling_group_id) {
return statistics{
bytes_sent[scheduling_group_id],
bytes_received[scheduling_group_id],
};
}

void posix_network_stack::clear_stats(unsigned scheduling_group_id) {
bytes_sent[scheduling_group_id] = 0;
bytes_received[scheduling_group_id] = 0;
}

}

}
19 changes: 19 additions & 0 deletions src/net/stack.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ module;
#ifdef SEASTAR_MODULE
module seastar;
#else
#include <seastar/core/metrics_api.hh>
#include <seastar/core/reactor.hh>
#include <seastar/net/stack.hh>
#include <seastar/net/inet_address.hh>
#endif
Expand Down Expand Up @@ -286,4 +288,21 @@ std::vector<network_interface> network_stack::network_interfaces() {
return {};
}

void register_net_metrics_for_scheduling_group(
metrics::metric_groups &metrics, unsigned sg_id, const metrics::label_instance& name) {
namespace sm = seastar::metrics;
metrics.add_group("network", {
sm::make_counter("bytes_sent", [sg_id] { return engine().net().stats(sg_id).bytes_sent; },
sm::description("Counts the number of bytes written to network sockets."), {name}),
sm::make_counter("bytes_received", [sg_id] { return engine().net().stats(sg_id).bytes_received; },
sm::description("Counts the number of bytes received from network sockets."), {name}),
});

// need to clear stats in case we recreated a SG with the same id
// but avoid during reactor startup
if (engine_is_ready()) {
engine().net().clear_stats(sg_id);
}
}

}