Skip to content

feat: Normalize OpenTelemetry configuration#11284

Closed
nfrankel wants to merge 1 commit into
masterfrom
feature/normalize_otel_config
Closed

feat: Normalize OpenTelemetry configuration#11284
nfrankel wants to merge 1 commit into
masterfrom
feature/normalize_otel_config

Conversation

@nfrankel

@nfrankel nfrankel commented May 23, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Regular OpenTelemetry Tracing uses environment variable for configuration. This way, Ops don't need to understand the details of each stack to configure OTEL.

Here's a sample Docker Compose snippet where each stack is implemented on a different technology. Note that all abide by the same environment variables.

  pricing:                      #python
    image: otel-pricing:1.1
    environment:
      OTEL_EXPORTER_OTLP_ENDPOINT: http://jaeger:4317
      OTEL_RESOURCE_ATTRIBUTES: service.name=pricing
      OTEL_METRICS_EXPORTER: none
      OTEL_LOGS_EXPORTER: none
  inventory:                 #rust
    image: otel-inventory:1.0
    environment:
      OTEL_EXPORTER_OTLP_ENDPOINT: http://jaeger:4317
      OTEL_RESOURCE_ATTRIBUTES: service.name=inventory
      OTEL_METRICS_EXPORTER: none
      OTEL_LOGS_EXPORTER: none
  warehouse-us:        #golang
    image: otel-warehouse-us:1.0
    environment:
      OTEL_EXPORTER_OTLP_ENDPOINT: http://jaeger:4317
      OTEL_RESOURCE_ATTRIBUTES: service.name=stocks USA
      OTEL_METRICS_EXPORTER: none
      OTEL_LOGS_EXPORTER: none
  warehouse-jp:      #ruby
    image: otel-warehouse-jp:1.0
    environment:
      OTEL_EXPORTER_OTLP_ENDPOINT: http://jaeger:4318
      OTEL_RESOURCE_ATTRIBUTES: service.name=stocks Japan
      OTEL_METRICS_EXPORTER: none
      OTEL_LOGS_EXPORTER: none

This PR is a starting point to allow Apache APISIX to follow the same conventions.

Two things of note, that should be discussed in this PR:

  • The default OTEL values defined in the spec are not the same as those defined by APISIX
  • The units defined in the spec are not the same as those defined by APISIX

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@nfrankel nfrankel added enhancement New feature or request discuss labels May 23, 2024
@nfrankel
nfrankel requested review from bzp2010 and membphis May 23, 2024 16:03
@nfrankel nfrankel changed the title feature: Normalize OpenTelemetry configuration feat: Normalize OpenTelemetry configuration May 23, 2024
@membphis

membphis commented May 28, 2024

Copy link
Copy Markdown
Member

a lots of CI failed, need to confirm what is the reason first

@shreemaan-abhishek do you have time to find out the reason?

@bzp2010

bzp2010 commented May 28, 2024

Copy link
Copy Markdown
Contributor

Looks like the default values are of the wrong type, perhaps they should be added " wrapped.
https://github.com/apache/apisix/actions/runs/9211255234/job/25340211929?pr=11284#step:19:483

@nfrankel
nfrankel force-pushed the feature/normalize_otel_config branch from 4a3023d to 498a82a Compare May 29, 2024 08:22
@nfrankel

nfrankel commented May 29, 2024

Copy link
Copy Markdown
Contributor Author

@membphis Tests are fixed

@bzp2010 bzp2010 mentioned this pull request Jun 8, 2024
Merged
5 tasks
@github-actions

github-actions Bot commented Jul 29, 2024

Copy link
Copy Markdown

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions Bot added the stale label Jul 29, 2024
@github-actions

github-actions Bot commented Aug 26, 2024

Copy link
Copy Markdown

This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions Bot closed this Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@membphis membphis Awaiting requested review from membphis
@bzp2010 bzp2010 Awaiting requested review from bzp2010

Assignees

No one assigned

Labels

discuss enhancement New feature or request stale user responded

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nfrankel @membphis @bzp2010