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

Add OTLP HTTP to gateway #948

Merged
merged 4 commits into from
Jun 3, 2024
Merged

Conversation

pavolloffay
Copy link
Collaborator

@pavolloffay pavolloffay commented May 31, 2024

Notable changes

  • Enables OTLP HTTP by default on gateway
  • microservices and monolithic
  • OTLP HTTP port is always first on the distributor service

@codecov-commenter
Copy link

codecov-commenter commented May 31, 2024

Codecov Report

Attention: Patch coverage is 89.47368% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 76.51%. Comparing base (26b93e5) to head (7fed685).
Report is 1 commits behind head on main.

Files Patch % Lines
internal/manifests/monolithic/configmap.go 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #948      +/-   ##
==========================================
- Coverage   76.55%   76.51%   -0.05%     
==========================================
  Files          94       94              
  Lines        6074     6071       -3     
==========================================
- Hits         4650     4645       -5     
- Misses       1152     1153       +1     
- Partials      272      273       +1     
Flag Coverage Δ
unittests 76.51% <89.47%> (-0.05%) ⬇️

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.

@pavolloffay
Copy link
Collaborator Author

My test manifests

kubectl apply -f - <<EOF
apiVersion: tempo.grafana.com/v1alpha1
kind: TempoMonolithic
metadata:
  name: sample
spec:
  multitenancy:
    enabled: true
    mode: openshift 
    authentication: 
      - tenantName: dev 
        tenantId: "1610b0c3-c509-4592-a256-a1871353dbfa" 
      - tenantName: prod
        tenantId: "1610b0c3-c509-4592-a256-a1871353dbfb"
  storage:
    traces:
      backend: memory
  resources:
    limits:
      cpu: "500m"
      memory: "2Gi"
  jaegerui:
    enabled: true
    route:
      enabled: true
    resources:
      limits:
        cpu: "500m"
        memory: "1Gi"
EOF

kubectl apply -f - <<EOF
apiVersion: tempo.grafana.com/v1alpha1
kind:  TempoStack
metadata:
  name: simplest
spec:
  managementState: Managed
  storage:
    secret:
      name: minio-test
      type: s3
  storageSize: 1Gi
  resources:
    total:
      limits:
        memory: 2Gi
        cpu: 2000m
  tenants:
    mode: openshift 
    authentication: 
      - tenantName: dev 
        tenantId: "1610b0c3-c509-4592-a256-a1871353dbfa" 
      - tenantName: prod
        tenantId: "1610b0c3-c509-4592-a256-a1871353dbfb"
  template:
    gateway:
      enabled: true 
    queryFrontend:
      jaegerQuery:
        enabled: true
EOF


kubectl apply -f - <<EOF
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: tempostack-traces-reader
rules:
  - apiGroups:
      - 'tempo.grafana.com'
    resources: 
      - dev
      - prod
    resourceNames:
      - traces
    verbs:
      - 'get' 
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: tempostack-traces-reader
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: tempostack-traces-reader
subjects:
  - kind: Group
    apiGroup: rbac.authorization.k8s.io
    name: system:authenticated
EOF


kubectl apply -f - <<EOF
apiVersion: v1
kind: ServiceAccount
metadata:
  name: otel-collector 
  namespace: ploffay
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: tempostack-traces-write
rules:
  - apiGroups:
      - 'tempo.grafana.com'
    resources: 
      - dev
    resourceNames:
      - traces
    verbs:
      - 'create' 
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: tempostack-traces
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: tempostack-traces-write
subjects:
  - kind: ServiceAccount
    name: otel-collector
    namespace: ploffay
EOF


kubectl apply -f - <<EOF
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: cluster-collector
  namespace: ploffay
spec:
  image: ghcr.io/open-telemetry/opentelemetry-collector-releases/opentelemetry-collector-contrib:0.101.0
  mode: deployment
  serviceAccount: otel-collector
  config: |
      extensions:
        bearertokenauth:
          filename: "/var/run/secrets/kubernetes.io/serviceaccount/token"
      receivers:
        otlp:
          protocols:
            http: {}
            grpc: {}
      processors:
        batch: {}
      exporters:
        debug: {}
        otlp/dev:
          endpoint: tempo-simplest-gateway.ploffay.svc.cluster.local:8090
          tls:
            insecure: false
            ca_file: "/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt"
          auth:
            authenticator: bearertokenauth
          headers:
            X-Scope-OrgID: "dev"
        otlphttp/dev:
          endpoint: https://tempo-sample-gateway.ploffay.svc.cluster.local:8080/api/traces/v1/dev
          tls:
            insecure: false
            ca_file: "/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt"
          auth:
            authenticator: bearertokenauth
          headers:
            X-Scope-OrgID: "dev"
      service:
        extensions: [bearertokenauth]
        pipelines:
          traces:
            receivers: [otlp]
            processors: [batch]
            exporters: [debug,otlphttp/dev]
EOF



@pavolloffay pavolloffay marked this pull request as ready for review May 31, 2024 15:03
@IshwarKanse
Copy link
Contributor

@pavolloffay We need to update the tests/e2e/gateway tests/e2e-openshift/multitenancy/ cases for the change.

--- a/tests/e2e-openshift/multitenancy/01-assert.yaml
+++ b/tests/e2e-openshift/multitenancy/01-assert.yaml
@@ -155,6 +155,7 @@ spec:
         - --web.listen=0.0.0.0:8080
         - --web.internal.listen=0.0.0.0:8081
         - --traces.write.otlpgrpc.endpoint=tempo-simplest-distributor.chainsaw-multitenancy.svc.cluster.local:4317
+        - --traces.write.otlphttp.endpoint=https://tempo-simplest-distributor.chainsaw-multitenancy.svc.cluster.local:4318

I tested the PR and the changes LGTM, I'm working on adding some additional steps to the tests and will submit the PR after this gets merged.

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
targetPort: 4317
- appProtocol: http
name: otlp-http-http
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use otlp-http and otlp-grpc as port names?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These ports names are set by the OTEL operator. They are derived from the receiver name e.g. otlp/http

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, sorry, I didn't realize this is from the OTEL collector.

@pavolloffay pavolloffay merged commit 9a0fe18 into grafana:main Jun 3, 2024
11 checks 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
Development

Successfully merging this pull request may close these issues.

4 participants