Opentelemetry exporter #531

KalmanMeth · 2023-10-24T10:34:53Z

Description

Export logs, traces, and metrics to an Opentelemetry collector.
Configuration enables to specify using either grpc or http protocol.
TLS details may be specified. If not specified, then the tls option is set to insecure.
Sample configuration:


  - name: otlp_collector
    encode:
      type: otlptraces
      otlptraces:
        address: localhost
        port: 4317
        connectionType: grpc
  - name: otlp_instana
    encode:
      type: otlptrace
      otlptrace:
        httpaddress: "1.2.3.4:443"
        tls:
          insecureSkipVerify: true
        headers:
          x-instana-key: *****
          x-instana-host: host
          x-instana-time: "0"

Headers to be sent to collector may be specified.
Field names should be transformed to Opentelemetry-conformant format using a transform_generic stage.


  - name: otlp_transform
    transform:
      type: generic
      generic:
        policy: replace_keys
        rules:
          - input: Bytes
            output: bytes
            multiplier: 1
          - input: Packets
            output: packets
          - input: DstAddr
            output: destination.address
          - input: DstMac
            output: destination.mac
          - input: DstHostIP
            output: destination.host.address
          - input: DstK8S_HostName
            output: destination.k8s.node.name
          - input: DstPort
            output: destination.port
          - input: DstPod
            output: destination.k8s.pod.name
          - input: DstK8S_Name
            output: destination.k8s.name
          - input: DstK8S_Type
            output: destination.k8s.type
          - input: DstK8S_OwnerName
            output: destination.k8s.owner.name
          - input: DstK8S_Type
            output: destination.k8s.type
          - input: DstK8S_HostIP
            output: destination.k8s.host.address
          - input: DstK8S_HostName
            output: destination.k8s.host.name
          - input: DstK8S_Namespace
            output: destination.k8s.namespace.name
          - input: SrcAddr
            output: source.address
          - input: SrcMac
            output: source.mac
          - input: SrcHostIP
            output: source.host.address
          - input: SrcK8S_HostName
            output: source.k8s.node.name
          - input: SrcPort
            output: source.port
          - input: SrcPod
            output: source.k8s.pod.name
          - input: SrcK8S_Name
            output: source.k8s.name
          - input: SrcK8S_Type
            output: source.k8s.type
          - input: SrcK8S_OwnerName
            output: source.k8s.owner.name
          - input: SrcK8S_OwnerType
            output: source.k8s.owner.type
          - input: SrcK8S_Namespace
            output: source.k8s.namespace.name
          - input: SrcK8S_HostIP
            output: source.k8s.host.address
          - input: SrcK8S_HostName
            output: source.k8s.host.name
          - input: TimeReceived
            output: timereceived
          - input: Proto
            output: protocol
          - input: FlowDirection
            output: flowdirection

Dependencies

n/a

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 · 2023-10-24T12:15:46Z

[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 kalmanmeth. 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

openshift-ci · 2023-10-24T12:15:53Z

[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 kalmanmeth. 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

codecov · 2023-10-24T12:26:30Z

Codecov Report

Attention: 283 lines in your changes are missing coverage. Please review.

Comparison is base (7c78d63) 66.84% compared to head (4a97ae5) 66.09%.
Report is 19 commits behind head on main.

Files	Patch %	Lines
pkg/pipeline/encode/opentelemetry/opentelemetry.go	54.39%	99 Missing and 10 partials ⚠️
...ipeline/encode/opentelemetry/encode_otlpmetrics.go	46.10%	80 Missing and 3 partials ⚠️
pkg/pipeline/transform/transform_network.go	25.00%	39 Missing and 3 partials ⚠️
.../pipeline/encode/opentelemetry/encode_otlptrace.go	32.20%	39 Missing and 1 partial ⚠️
pkg/pipeline/encode/encode_prom_metric.go	76.92%	3 Missing ⚠️
pkg/pipeline/transform/kubernetes/kubernetes.go	0.00%	3 Missing ⚠️
pkg/api/encode_prom.go	66.66%	2 Missing ⚠️
pkg/pipeline/pipeline_builder.go	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #531      +/-   ##
==========================================
- Coverage   66.84%   66.09%   -0.76%     
==========================================
  Files          95      102       +7     
  Lines        6802     7409     +607     
==========================================
+ Hits         4547     4897     +350     
- Misses       1988     2221     +233     
- Partials      267      291      +24

Flag	Coverage Δ
unittests	`66.09% <50.35%> (-0.76%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jotak · 2023-10-31T17:20:03Z

Thanks @KalmanMeth !
On semantic convention, I wonder if we should have something more "baked-in" rather than asking the user to configure the translations. Especially, some stuff might be hard to do only by config, e.g: our SrcK8S_Name should actually be translated into source.k8s.pod.name or source.k8s.service.name or source.k8s.node.name depending on what is SrcK8S_Type.

So perhaps, since the knowledge currently lies in the add_kubernetes transform, we could have a flag otel on/off in this block that, if turned on, would automatically use the otel semantics instead of our current names ?

KalmanMeth · 2023-11-20T12:04:10Z

Thanks @KalmanMeth ! On semantic convention, I wonder if we should have something more "baked-in" rather than asking the user to configure the translations. Especially, some stuff might be hard to do only by config, e.g: our SrcK8S_Name should actually be translated into source.k8s.pod.name or source.k8s.service.name or source.k8s.node.name depending on what is SrcK8S_Type.

So perhaps, since the knowledge currently lies in the add_kubernetes transform, we could have a flag otel on/off in this block that, if turned on, would automatically use the otel semantics instead of our current names ?

I added the otel option to add_kubernetes by setting the assignee field to otel. I defined the fields like k8s.node.name, k8s.pod.name, etc. We should have a discussion on which other fields we think we should provide in this manner as opposed to using the transform-generic capabilities.

KalmanMeth · 2023-11-20T12:20:31Z

I have working counters and gauges for the opentelemetry metrics. I removed the code for opentelemetry historgrams. This may have to wait for a future PR.

jotak · 2023-11-21T14:03:16Z

pkg/pipeline/encode/opentelemetry/encode_otlptrace.go

+func (e *EncodeOtlpTrace) Encode(metricRecord config.GenericMap) {
+	log.Tracef("entering EncodeOtlpTrace. entry = %v", metricRecord)
+	_, span := e.tracer.Start(e.ctx, flpEncodeSpanName)
+	defer span.End()
+	attributes := obtainAttributesFromEntry(metricRecord)
+	span.SetAttributes(*attributes...)
+}


I am not sure what is the goal here with traces, but I don't think we're doing something useful here.
Normally, traces are provided by the workloads that are being traced, for instance when a workload A sends a request to service S, A would generate a span saying "I am sending a request to S".

I believe what we could possibly do, as an external observer, is to generate spans on behalf of the workloads & services as long as we see their IPs flowing. I'm not 100% sure how useful it is, but that's the best I can see. But it's not really what this is doing here: it creates spans for FLP without providing any real information about the network connections being observed in otel format.

jotak · 2023-11-21T15:04:15Z

pkg/pipeline/encode/opentelemetry/encode_otlpmetrics.go

+		case "nil":
+			if found {
+				return nil
+			}
+		case "!nil":
+			if !found {
+				return nil
+			}
+		default:


This code that came from encode_prom is obsolete, see https://github.com/netobserv/flowlogs-pipeline/pull/513/files#diff-30d11303a4ad1ec82e4f39e3ae6dc7d044c8dd370c536a98ba90febbfe2d425aR209

basically: these special keywords "nil" / "!nil" have been replaced with a more explicit API where you can have more advanced filters, e.g. using regexp

jotak · 2023-11-21T15:19:42Z

Focusing on code reviewing strictly speaking rather than if it meets expectations because I must say I am not quite sure about the use cases but you @KalmanMeth have apparently one using instana so you're you own judge on that :-) I struggle a bit to see where netobserv can bring value on tracing (unless we captured http headers with our agents, which we don't)

So apart from this comment this looks good to me.

I guess there can be follow-ups to improve more the semantic conventions compliance.

KalmanMeth · 2024-01-02T10:32:50Z

@jotak The trace capability now breaks down a single log entry into sub-entries - one for source and one for destination. I added instructions to test the setup using the opentelemetry collector and jaeger tracing UI that is fed with information from flp. I would like to progress with this PR to merge.

jotak

lgtm
thanks @KalmanMeth !

jotak · 2024-01-09T08:21:25Z

/lgtm

OlivierCazade · 2024-01-17T16:37:03Z

Adding breaking change label due to change in name type in the API

openshift-merge-robot added the needs-rebase label Oct 24, 2023

KalmanMeth closed this Oct 24, 2023

KalmanMeth force-pushed the opentelemetry branch from bbbde61 to 756cb79 Compare October 24, 2023 11:41

KalmanMeth reopened this Oct 24, 2023

openshift-merge-robot removed the needs-rebase label Oct 24, 2023

KalmanMeth force-pushed the opentelemetry branch from 1525128 to a2aaf87 Compare November 2, 2023 14:12

KalmanMeth changed the title ~~Opentelemetry exporter~~ WIP: Opentelemetry exporter Nov 2, 2023

openshift-ci bot added the do-not-merge/work-in-progress label Nov 2, 2023

openshift-merge-robot added the needs-rebase label Nov 11, 2023

KalmanMeth force-pushed the opentelemetry branch from a2aaf87 to 912c0ac Compare November 15, 2023 12:07

openshift-merge-robot removed the needs-rebase label Nov 15, 2023

KalmanMeth changed the title ~~WIP: Opentelemetry exporter~~ Opentelemetry exporter Nov 20, 2023

openshift-ci bot removed the do-not-merge/work-in-progress label Nov 20, 2023

jotak reviewed Nov 21, 2023

View reviewed changes

KalmanMeth added 7 commits November 22, 2023 10:51

added otlp encode stages

f8a289e

added some vendor files

c6dffed

restored encode_ptlp.go

2265a4d

removed unfinished code for histograms

c719aab

added otel support inside transform-network

6c6b43d

removed redundant definition of metricsProcessed metric

879b890

update otel metric to changes in prom metric

1d4010a

KalmanMeth force-pushed the opentelemetry branch from 8eea719 to 1d4010a Compare November 22, 2023 09:32

KalmanMeth added 10 commits November 22, 2023 15:48

separated spans for src and dst

05aece8

create child span only when selector field was present

835f56a

added errorCounter operational metric to otel

954775e

refactored metrics items

574607a

minor edits

f21b4e3

removed unused import

601e9fc

changed flp to netobserv

3bdbafb

added instructions for demo

4c74cef

updated demo doc

68b65b2

updated demo.md

4a97ae5

jotak approved these changes Jan 9, 2024

View reviewed changes

jotak added no-qe This PR doesn't necessitate QE approval no-doc This PR doesn't require documentation change on the NetObserv operator labels Jan 9, 2024

KalmanMeth merged commit 31e4180 into netobserv:main Jan 9, 2024
7 of 8 checks passed

OlivierCazade added the breaking-change This pull request has breaking changes. They should be described in PR description. label Jan 17, 2024

KalmanMeth mentioned this pull request Feb 20, 2024

Create new transformer into OpenTelemetry semantics #311

Closed

jpinsonneau mentioned this pull request Jun 6, 2024

NETOBSERV-1522 Open Telemetry exporter API netobserv/network-observability-operator#671

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Opentelemetry exporter #531

Opentelemetry exporter #531

KalmanMeth commented Oct 24, 2023 •

edited

Loading

openshift-ci bot commented Oct 24, 2023

openshift-ci bot commented Oct 24, 2023

codecov bot commented Oct 24, 2023 •

edited

Loading

jotak commented Oct 31, 2023

KalmanMeth commented Nov 20, 2023

KalmanMeth commented Nov 20, 2023

jotak Nov 21, 2023

jotak Nov 21, 2023 •

edited

Loading

jotak Nov 21, 2023

KalmanMeth Nov 22, 2023

jotak commented Nov 21, 2023

KalmanMeth commented Jan 2, 2024

jotak left a comment

jotak commented Jan 9, 2024

OlivierCazade commented Jan 17, 2024

Opentelemetry exporter #531

Opentelemetry exporter #531

Conversation

KalmanMeth commented Oct 24, 2023 • edited Loading

Description

Dependencies

Checklist

openshift-ci bot commented Oct 24, 2023

openshift-ci bot commented Oct 24, 2023

codecov bot commented Oct 24, 2023 • edited Loading

Codecov Report

jotak commented Oct 31, 2023

KalmanMeth commented Nov 20, 2023

KalmanMeth commented Nov 20, 2023

jotak Nov 21, 2023

Choose a reason for hiding this comment

jotak Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

jotak Nov 21, 2023

Choose a reason for hiding this comment

KalmanMeth Nov 22, 2023

Choose a reason for hiding this comment

jotak commented Nov 21, 2023

KalmanMeth commented Jan 2, 2024

jotak left a comment

Choose a reason for hiding this comment

jotak commented Jan 9, 2024

OlivierCazade commented Jan 17, 2024

KalmanMeth commented Oct 24, 2023 •

edited

Loading

codecov bot commented Oct 24, 2023 •

edited

Loading

jotak Nov 21, 2023 •

edited

Loading