NETOBSERV-1522 Open Telemetry exporter API #671

jpinsonneau · 2024-06-06T10:22:24Z

Description

Add Open Telemetry to FlowCollector exporters:

apiVersion: flows.netobserv.io/v1beta2
kind: FlowCollector
metadata:
  name: cluster
spec:
...
  exporters:
    - type: OpenTelemetry
      openTelemetry:
        targetHost: my-otelcol-collector-headless.otlp.svc
        targetPort: 4317
        type: grpc
        logs:
          enable: true
        metrics:
          enable: true
          prefix: netobserv
          pushTimeInterval: 20s
          expiryTime: 2m

Custom transform rules can be applied using customRules field in FlowCollectorOpenTelemetry exporter. Else default rules from https://github.com/rhobs/observability-data-model/blob/main/network-observability.md#format-proposal are applied.
See OpenTelemetryDefaultTransformRules in constants.

Rely on https://github.com/netobserv/flowlogs-pipeline/blob/main/contrib/opentelemetry/demo.md for Otel installation on your cluster

TODO: dig into ways to implement traces. Current traces configuration is disabled to avoid confusion.

Dependencies

netobserv/flowlogs-pipeline#672
netobserv/flowlogs-pipeline#684

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.

openshift-ci · 2024-06-06T10:22:28Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

jpinsonneau · 2024-06-06T10:34:52Z

apis/flowcollector/v1beta2/flowcollector_types.go

+	TLS ClientTLS `json:"tls"`
+
+	// Rules to transform flow logs to Opentelemetry-conformant format.
+	Rules []api.GenericTransformRule `json:"rules,omitempty"`


Should we expose rules or hardcode them ?

See netobserv/flowlogs-pipeline#531 for FLP implementation

jpinsonneau · 2024-06-10T16:09:46Z

controllers/flp/flp_pipeline_builder.go

+		fromStage.EncodeOtelMetrics(fmt.Sprintf("%s-metrics", name), api.EncodeOtlpMetrics{
+			OtlpConnectionInfo: &conn,
+			Prefix:             constants.OperatorName,
+			Metrics:            flpMetrics,


☝️ we should probably adapt the metrics as same as logs to match Open Telemetry field namings

For now I just got rid of the transform stage and kept the same metrics as prometheus. Let me know your thoughts

+1, that means to rewrite flpMetrics to map all the labels and the filters with the otel semantic.
That can be done in a follow-up I guess, if you prefer

Sure I can add a todo

done in 6b556fb

jotak · 2024-06-17T11:29:59Z

apis/flowcollector/v1beta2/flowcollector_types.go

+
+	// Type of Open Telemetry connection. The available options are `http` and `grpc`.
+	// +optional
+	Type string `json:"type"`


Can we rename protocol ?
That's the term used in otel: https://opentelemetry.io/docs/collector/configuration/

jotak · 2024-06-17T11:57:51Z

apis/flowcollector/v1beta2/flowcollector_types.go

+
+	// Time duration of no-flow to wait before deleting data item
+	// +kubebuilder:default:="2m"
+	ExpiryTime *metav1.Duration `json:"expiryTime,omitempty"`


the same setting exists for regular prom metrics but we chose not to expose it as this is more of an implementation detail. I think we should do the same here ;

or if we really want to expose it, then it might be better to set as an advanced setting, and even share the same between prom & otel configs (because at the end of the day this is really the same thing: how long do we keep in memory metrics that reference elements that may disappear, such as those having a pod name label and that pod doesn't exist anymore)

But for simplicity I wouldn't even bother about that, and just remove this setting

Ok I'll remove it then

Should we keep the pushTimeInterval ?

jotak · 2024-06-17T12:03:33Z

apis/flowcollector/v1beta2/flowcollector_types.go

+
+	// Prefix added to each metric name
+	// +optional
+	Prefix string `json:"prefix,omitempty"`


like "expiryTime", there's also a "prefix" config that we force as "netobserv" for regular metrics, and I wonder if we really should expose it, or keep being more opinionated. I tend to prefer being more opinionated, and expose less config, at least as long as nobody has a need for it

Sure. I can remove it and force netobserv here then

jotak · 2024-06-17T12:05:44Z

apis/flowcollector/v1beta2/flowcollector_types.go

@@ -16,6 +16,7 @@ limitations under the License.
 package v1beta2

 import (
+	"github.com/netobserv/flowlogs-pipeline/pkg/api"


we should not have coupling with the FLP api here, the config definitions should be duplicated (or if we force the semantic convention then we can just remove that)

I duplicated the GenericTransformRule

jpinsonneau · 2024-06-17T14:17:26Z

Addressed feedback & updated go vendor

jpinsonneau · 2024-07-01T14:16:32Z

As this PR is still waiting for review, I took the chance to do the metrics transformation and added testing
6b556fb

jotak · 2024-07-01T14:36:20Z

apis/flowcollector/v1beta2/flowcollector_types.go

+	TargetPort int `json:"targetPort"`
+
+	// Protocol of Open Telemetry connection. The available options are `http` and `grpc`.
+	// +optional


it should be an enum I guess

sure, efdb0d4

jotak · 2024-07-01T15:02:15Z

apis/flowcollector/v1beta2/flowcollector_types.go

+	// Custom rules to transform flow logs to Opentelemetry-conformant format.
+	// By default the following format will be used: https://github.com/rhobs/observability-data-model/blob/main/network-observability.md#format-proposal
+	// Setting `customRules` will fully override the default rules allowing you to export each field to your naming convention
+	// +optional
+	Rules *[]GenericTransformRule `json:"customRules,omitempty"`


In general we try to make the CRD more opinionated than what FLP exposes. Here, I think we should present that more clearly as a mapping of fields, and tell why we expose it (because there's no standard)

Here's a proposal:

Suggested change

// Custom rules to transform flow logs to Opentelemetry-conformant format.

// By default the following format will be used: https://github.com/rhobs/observability-data-model/blob/main/network-observability.md#format-proposal

// Setting `customRules` will fully override the default rules allowing you to export each field to your naming convention

// +optional

Rules *[]GenericTransformRule `json:"customRules,omitempty"`

// Custom fields mapping to an OpenTelemetry conformant format.

// By default, NetObserv format proposal is used: https://github.com/rhobs/observability-data-model/blob/main/network-observability.md#format-proposal .

// As there is currently no accepted otlp standard for L3/4 network logs, you can freely override it with your own.

// +optional

FieldsMapping *[]GenericTransformRule `json:"fieldsMapping,omitempty"`

also done in efdb0d4

jotak

thanks, lgtm!

memodi · 2024-07-09T14:21:29Z

/ok-to-test

github-actions · 2024-07-09T14:25:02Z

New images:

quay.io/netobserv/network-observability-operator:cd31dfd
quay.io/netobserv/network-observability-operator-bundle:v0.0.0-cd31dfd
quay.io/netobserv/network-observability-operator-catalog:v0.0.0-cd31dfd

They will expire after two weeks.

To deploy this build:

# Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:cd31dfd make deploy

# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-cd31dfd

Or as a Catalog Source:

apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
  name: netobserv-dev
  namespace: openshift-marketplace
spec:
  sourceType: grpc
  image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-cd31dfd
  displayName: NetObserv development catalog
  publisher: Me
  updateStrategy:
    registryPoll:
      interval: 1m

openshift-ci · 2024-07-10T14:19:41Z

New changes are detected. LGTM label has been removed.

openshift-ci · 2024-07-10T14:19:49Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from jotak. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jpinsonneau · 2024-07-10T14:20:08Z

Fixed empty metrics in 9c9c293

memodi · 2024-07-12T16:23:35Z

/ok-to-test

github-actions · 2024-07-12T16:26:34Z

New images:

quay.io/netobserv/network-observability-operator:809a682
quay.io/netobserv/network-observability-operator-bundle:v0.0.0-809a682
quay.io/netobserv/network-observability-operator-catalog:v0.0.0-809a682

They will expire after two weeks.

To deploy this build:

# Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:809a682 make deploy

# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-809a682

Or as a Catalog Source:

apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
  name: netobserv-dev
  namespace: openshift-marketplace
spec:
  sourceType: grpc
  image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-809a682
  displayName: NetObserv development catalog
  publisher: Me
  updateStrategy:
    registryPoll:
      interval: 1m

github-actions · 2024-07-16T06:01:11Z

New images:

quay.io/netobserv/network-observability-operator:eb79fe5
quay.io/netobserv/network-observability-operator-bundle:v0.0.0-eb79fe5
quay.io/netobserv/network-observability-operator-catalog:v0.0.0-eb79fe5

They will expire after two weeks.

To deploy this build:

# Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:eb79fe5 make deploy

# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-eb79fe5

Or as a Catalog Source:

apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
  name: netobserv-dev
  namespace: openshift-marketplace
spec:
  sourceType: grpc
  image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-eb79fe5
  displayName: NetObserv development catalog
  publisher: Me
  updateStrategy:
    registryPoll:
      interval: 1m

jpinsonneau · 2024-07-16T06:01:13Z

Fix the value key mentionned in otel metrics config
7ba86fe

openshift-ci bot added the do-not-merge/work-in-progress label Jun 6, 2024

jpinsonneau commented Jun 6, 2024

View reviewed changes

jpinsonneau force-pushed the 1522 branch 3 times, most recently from 41f1e80 to 6a1582e Compare June 10, 2024 13:48

jpinsonneau changed the title ~~NETOBSERV-1522 wip otel exporter API~~ NETOBSERV-1522 Open Telemetry exporter API Jun 10, 2024

jpinsonneau marked this pull request as ready for review June 10, 2024 14:20

openshift-ci bot removed the do-not-merge/work-in-progress label Jun 10, 2024

jpinsonneau requested review from jotak, msherif1234 and OlivierCazade June 10, 2024 14:20

jpinsonneau force-pushed the 1522 branch from e92e117 to baa1bb6 Compare June 10, 2024 14:39

jpinsonneau commented Jun 10, 2024

View reviewed changes

jotak reviewed Jun 17, 2024

View reviewed changes

openshift-merge-robot added the needs-rebase label Jun 26, 2024

jpinsonneau added 4 commits June 26, 2024 15:45

otel exporter API

d91053d

addressed feedback

94d0fcb

update vendor

317379b

update flp

d7a12c1

jpinsonneau force-pushed the 1522 branch from fab2d20 to d7a12c1 Compare June 26, 2024 13:50

openshift-merge-robot removed the needs-rebase label Jun 26, 2024

jpinsonneau added 2 commits June 26, 2024 15:55

update bundle

6d16b88

metrics transformation + tests

6b556fb

jotak reviewed Jul 1, 2024

View reviewed changes

addressed feedback

efdb0d4

jotak approved these changes Jul 2, 2024

View reviewed changes

openshift-ci bot assigned jotak Jul 2, 2024

openshift-ci bot added the lgtm label Jul 2, 2024

openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jul 9, 2024

fix metrics

9c9c293

openshift-ci bot removed the lgtm label Jul 10, 2024

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jul 10, 2024

fix metrics prefix

01dde44

openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jul 12, 2024

convert value keys

7ba86fe

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jul 16, 2024

jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jul 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NETOBSERV-1522 Open Telemetry exporter API #671

NETOBSERV-1522 Open Telemetry exporter API #671

jpinsonneau commented Jun 6, 2024 •

edited

Loading

openshift-ci bot commented Jun 6, 2024

jpinsonneau Jun 6, 2024

jpinsonneau Jun 10, 2024

jotak Jun 17, 2024

jpinsonneau Jun 17, 2024

jpinsonneau Jul 1, 2024

jotak Jun 17, 2024

jpinsonneau Jun 17, 2024

jotak Jun 17, 2024 •

edited

Loading

jpinsonneau Jun 17, 2024

jpinsonneau Jun 17, 2024

jotak Jun 17, 2024

jpinsonneau Jun 17, 2024

jotak Jun 17, 2024

jpinsonneau Jun 17, 2024

jpinsonneau commented Jun 17, 2024

jpinsonneau commented Jul 1, 2024

jotak Jul 1, 2024

jpinsonneau Jul 2, 2024

jotak Jul 1, 2024

jpinsonneau Jul 2, 2024

jotak left a comment

memodi commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

openshift-ci bot commented Jul 10, 2024

openshift-ci bot commented Jul 10, 2024

jpinsonneau commented Jul 10, 2024

memodi commented Jul 12, 2024

github-actions bot commented Jul 12, 2024

github-actions bot commented Jul 16, 2024

jpinsonneau commented Jul 16, 2024

NETOBSERV-1522 Open Telemetry exporter API #671

Are you sure you want to change the base?

NETOBSERV-1522 Open Telemetry exporter API #671

Conversation

jpinsonneau commented Jun 6, 2024 • edited Loading

Description

Dependencies

Checklist

openshift-ci bot commented Jun 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jotak Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpinsonneau commented Jun 17, 2024

jpinsonneau commented Jul 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jotak left a comment

Choose a reason for hiding this comment

memodi commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

openshift-ci bot commented Jul 10, 2024

openshift-ci bot commented Jul 10, 2024

jpinsonneau commented Jul 10, 2024

memodi commented Jul 12, 2024

github-actions bot commented Jul 12, 2024

github-actions bot commented Jul 16, 2024

jpinsonneau commented Jul 16, 2024

jpinsonneau commented Jun 6, 2024 •

edited

Loading

jotak Jun 17, 2024 •

edited

Loading