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.

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

Provide eBPF based Access Log Service #88

Merged
merged 5 commits into from
Dec 11, 2023

Conversation

mrproliu
Copy link
Contributor

No description provided.

ebpf/monitor/accesslog.proto Outdated Show resolved Hide resolved
ebpf/monitor/accesslog.proto Outdated Show resolved Hide resolved
}

message AccessLogHTTPProtocolResponse {
int32 statusCode = 1;
Copy link
Member

Choose a reason for hiding this comment

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

I remember HTTP2 response codes are not only number? Could you confirm?

Comment on lines 328 to 331
oneof timestamp {
EBPFOffsetTimestamp offset = 1;
Instant absolute = 2;
}
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain how this timestamp works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the kernel, we can get different timestamps. In the offset mode, we can only get the timestamp offset(through bpf_ktime_get_ns), and then we need to add it with the system boot time to get the real-time, that's why I added the system boot time in the node info. But in some kernel methods, we can only get the absolute timestamp.

Copy link
Member

Choose a reason for hiding this comment

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

Is the timestamp sent as always in a bulk at least?

ebpf/monitor/accesslog.proto Outdated Show resolved Hide resolved
}

message EBPFAccessLogNodeNetInterface {
int32 index = 1;
Copy link
Member

Choose a reason for hiding this comment

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

What is the index?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the network index. We can use the ip link show in Linux to get this information. When the stream has been created or the network device has been updated, I will add this data to the node info.
image

Comment on lines 40 to 41
// kernel level metrics
repeated AccessLogKernelLog kernelLogs = 3;
Copy link
Member

Choose a reason for hiding this comment

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

Is the kernel log with HTTP log?

Copy link
Member

Choose a reason for hiding this comment

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

What is the period and moment of sending TCP relative logs?

Copy link
Contributor Author

@mrproliu mrproliu Dec 11, 2023

Choose a reason for hiding this comment

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

If we cannot detect the kernel related to the application logs, then they should be sent through a timer.
Otherside, the kernel logs should related with a single application log.

Comment on lines 24 to 25
option java_package = "org.apache.skywalking.apm.network.ebpf.monitor.accesslog.v3";
option go_package = "skywalking.apache.org/repo/goapi/collect/ebpf/monitor/accesslog/v3";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
option java_package = "org.apache.skywalking.apm.network.ebpf.monitor.accesslog.v3";
option go_package = "skywalking.apache.org/repo/goapi/collect/ebpf/monitor/accesslog/v3";
option java_package = "org.apache.skywalking.apm.network.ebpf.accesslog.v3";
option go_package = "skywalking.apache.org/repo/goapi/collect/ebpf/accesslog/v3";

I think we don't need monitor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.

Copy link
Contributor Author

@mrproliu mrproliu Dec 11, 2023

Choose a reason for hiding this comment

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

I have moved this proto file into the ebpf directory.

@wu-sheng wu-sheng merged commit eea4e57 into apache:master Dec 11, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants