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

OpenTelemetry Protocol (OTLP) appender #374

Closed
wants to merge 8 commits into from

Conversation

devurandom
Copy link
Contributor

@devurandom devurandom commented Jun 19, 2023

Requires com.github.steffan-westcott/clj-otel-api.

With Java Agent

Activate an appender configured by the OpenTelemetry Java Agent:

(let [logger-provider (.getLogsBridge (GlobalOpenTelemetry/get))
      appender (taoensso.timbre.appenders.community.otlp/otlp-appender logger-provider)]
  (taoensso.timbre/merge-config! {:appenders {:otlp appender}}))

Note: When relying on the OpenTelemetry Java Agent 1.x, you need
to explicitly enable the logs exporter with OTEL_LOGS_EXPORTER=otlp.
This will become the default with the release of Java Agent 2.0, cf.

Without Java Agent

If you want autoconfiguration without the Java Agent, you also need
io.opentelemetry/opentelemetry-sdk-extension-autoconfigure and
io.opentelemetry/opentelemetry-exporter-otlp on the classpath.

Create an autoconfigured appender and activate it:

(let [logger-provider (.getSdkLoggerProvider
                        (.getOpenTelemetrySdk
                          (.build
                            (AutoConfiguredOpenTelemetrySdk/builder))))
      appender (taoensso.timbre.appenders.community.otlp/otlp-appender logger-provider)]
  (taoensso.timbre/merge-config! {:appenders {:otlp appender}}))

If you already have an instance of GlobalOpenTelemetry, e.g. created
by the OpenTelemetry Java Agent, you need to prevent setting the newly
created SDK as the global default:

(.build
  (doto (AutoConfiguredOpenTelemetrySdk/builder)
    (.setResultAsGlobal false)))

I took inspiration from taoensso.timbre.appenders.community.sentry.

Once steffan-westcott/clj-otel#8 is implemented,
the actual log emission should be replaced with using clj-otel's API.

ptaoussanis and others added 6 commits February 27, 2023 17:11
Changes:

  - No longer throw on invalid compile-time :min-level
  - Add `:_init-config` map to `*config*` to help users understand/debug
    load-time config
Libraries printing information without control are often a nuisance.
@devurandom devurandom force-pushed the ds/otlp-appender branch 2 times, most recently from 62a90ad to f47e9a8 Compare June 19, 2023 13:04
Requires com.github.steffan-westcott/clj-otel-api.

With Java Agent:

Activate an appender configured by the OpenTelemetry Java Agent:
```clj
(let [logger-provider (.getLogsBridge (GlobalOpenTelemetry/get))
      appender (taoensso.timbre.appenders.community.otlp/otlp-appender logger-provider)]
  (taoensso.timbre/merge-config! {:appenders {:otlp appender}}))
```

Note: When relying on the OpenTelemetry Java Agent 1.x, you need
to explicitly enable the logs exporter with `OTEL_LOGS_EXPORTER=otlp`.
This will become the default with the release of Java Agent 2.0, cf.
* https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/CHANGELOG.md#version-1270-2023-06-14
* open-telemetry/opentelemetry-java-instrumentation#8647

Without Java Agent:

If you want autoconfiguration without the Java Agent, you also need
io.opentelemetry/opentelemetry-sdk-extension-autoconfigure and
io.opentelemetry/opentelemetry-exporter-otlp on the classpath.

Create an autoconfigured appender and activate it:
```clj
(let [logger-provider (.getSdkLoggerProvider
                        (.getOpenTelemetrySdk
                          (.build
                            (AutoConfiguredOpenTelemetrySdk/builder))))
      appender (taoensso.timbre.appenders.community.otlp/otlp-appender logger-provider)]
  (taoensso.timbre/merge-config! {:appenders {:otlp appender}}))
```

If you already have an instance of `GlobalOpenTelemetry`, e.g. created
by the OpenTelemetry Java Agent, you need to prevent setting the newly
created SDK as the global default:
```clj
(.build
  (doto (AutoConfiguredOpenTelemetrySdk/builder)
    (.setResultAsGlobal false)))
```

I took inspiration from `taoensso.timbre.appenders.community.sentry`.

Once steffan-westcott/clj-otel#8 is implemented,
the actual log emission should be replaced with using clj-otel's API.
assoc-some-nx is like `taoensso.encore/assoc-nx` but using
`taoensso.encore/assoc-some` instead of `clojure.core/assoc`.
@ptaoussanis
Copy link
Member

@devurandom Hi Dennis, thanks for this! Will take a proper look next time I'm doing batched work on Timbre 👍 No need to worry about rebasing, I'll take care of that during review.

@ptaoussanis ptaoussanis force-pushed the master branch 2 times, most recently from 51a3b72 to 01513d4 Compare February 22, 2024 16:40
@devurandom
Copy link
Contributor Author

@ptaoussanis Hi Peter! Is there anything I can do to help get this merged?

ptaoussanis pushed a commit that referenced this pull request Feb 23, 2024
…evurandom)

Once <steffan-westcott/clj-otel#8> is implemented,
the actual log emission should be replaced with clj-otel's API.
@ptaoussanis
Copy link
Member

@devurandom Hi Dennis, apologies for the delay! I would have included this in yesterday's update (v6.4.0), but that was a bit of a rush and mostly intended as a hotfix release.

I've manually merged your PR here, thanks for all the work on this! 🙏

I made a few superficial changes, just to try keep a ~consistent style between the various appenders.

Please let me know if you're happy with that commit, in which case I'll aim to cut a new release in the next few days.

Cheers! :-)

@devurandom
Copy link
Contributor Author

devurandom commented Feb 26, 2024

Hi Peter! Thanks! I looked at git diff 39c6e310a62013b694116fd1c66db5b5eb3cd6c0..0bf280901f1174ac7c783dc2f5ff1f8e1ba32e1a -- src/taoensso/timbre/appenders/community/otlp.clj to review the changes. The code looks good and I noticed no issues.

Re: For agent v2.x: the logs exporter should be enabled by default [1].

By now v2.0 of the Java SDK has been released, so we could just link to its changelog (instead of linking to a GH issue and the changelog with the early notification): https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/v2.0.0/CHANGELOG.md#version-200-2024-01-12

Is there a particular reason you use URL shorteners for links in comments? I imagine they would be inconvenient if they ever become unavailable. It would be hard to figure out what they linked to and also archives would probably only have indexed the original link, but not the shortened one.

ptaoussanis pushed a commit that referenced this pull request Feb 26, 2024
…evurandom)

Once <steffan-westcott/clj-otel#8> is implemented,
the actual log emission should be replaced with clj-otel's API.
@ptaoussanis ptaoussanis force-pushed the master branch 2 times, most recently from 24cb3e9 to b72cc65 Compare February 26, 2024 13:36
@ptaoussanis
Copy link
Member

ptaoussanis commented Feb 26, 2024

Thanks for the review!

Closing, this has now included as part of the v6.5.0 release 👍

Final commit: f3ce2b5

@devurandom devurandom deleted the ds/otlp-appender branch February 26, 2024 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants