-
Notifications
You must be signed in to change notification settings - Fork 32
NETOBSERV-584: Support for writing logs to Loki (distributor) using gRPC #1086
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1086 +/- ##
==========================================
+ Coverage 65.77% 65.98% +0.21%
==========================================
Files 121 121
Lines 8938 9053 +115
==========================================
+ Hits 5879 5974 +95
- Misses 2729 2743 +14
- Partials 330 336 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=232d236 make set-flp-image |
@leandroberetta: This pull request references NETOBSERV-584 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
c2a742e
to
53fc1fe
Compare
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=fecd9f7 make set-flp-image |
pkg/api/write_loki.go
Outdated
MaxSendMsgSize int `yaml:"maxSendMsgSize,omitempty" json:"maxSendMsgSize,omitempty" doc:"maximum message size the client can send"` | ||
KeepAlive string `yaml:"keepAlive,omitempty" json:"keepAlive,omitempty" doc:"keep alive interval"` | ||
KeepAliveTimeout string `yaml:"keepAliveTimeout,omitempty" json:"keepAliveTimeout,omitempty" doc:"keep alive timeout"` | ||
TLS *GRPCTLSConfig `yaml:"tls,omitempty" json:"tls,omitempty" doc:"TLS configuration"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, we may reuse ClientConfig
that contains TLSConfig
?
https://github.com/prometheus/common/blob/v0.66.1/config/http_config.go#L1113
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checked that, surely that's doable, but prom's TLS config contains more advanced settings (e.g. min / max versions) which we would need to support in our implementation in the loki-client, if we want to be consistent with the proposed settings. Or we would need to reuse prom's implementation but I'm not sure they have grpc in the go-client. So perhaps having our own struct is good enough for now, after all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait, actually, we do also have another TLS struct of our own: api.ClientTLS
That one could be reused easily, no ?
/hold
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I've just seen this comment, actually, I did the work to reuse the prom's tls config. Also, I've made the necessary changes in the loki-client-go at netobserv/loki-client-go#4
@jotak the one you mentioned is used for Kafka and it seemed to be more simple than the prom one, for example, for now, communicating with the distributor in grpc mode, I need to configure mTLS which I don't see that is possible with the kafka one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, cool, that should be even better
go.mod
Outdated
|
||
replace github.com/vmware/go-ipfix => github.com/jotak/go-ipfix v0.0.0-20250708115123-407c539ea101 | ||
|
||
replace github.com/netobserv/loki-client-go => github.com/leandroberetta/loki-client-go v0.0.0-20250924145641-591b5499691d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can now remove that as this was merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
thanks @leandroberetta !
[edit] actually, last-minute comment: #1086 (comment)
e18125e
to
0e34623
Compare
0e34623
to
d01e386
Compare
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=ca38bdc make set-flp-image |
Thanks! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jotak The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
/test ci/prow/qe-e2e-tests |
@Amoghrd: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/test qe-e2e-tests |
@leandroberetta: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
@memodi question, I'm noticing the e2e tests are timeouting, I couldn't find more information in the logs. Extending the time might be a possible solution?. |
indeed, the default timeout is 2h, we could increase it like it has here: https://github.com/openshift/release/blob/master/ci-operator/config/netobserv/flowlogs-pipeline/netobserv-flowlogs-pipeline-main.yaml#L99 , would you mind raising a PR? |
Sure, openshift/release#70336 Let me know if I need to do something else, I couldn't assign it to you. First PR there :) |
Description
Support for writing logs to Loki (distributor) using gRPC
Ref: https://issues.redhat.com/browse/NETOBSERV-584
Dependencies
netobserv/loki-client-go#3
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.
To run a perfscale test, comment with:
/test flp-node-density-heavy-25nodes