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

Migrate to sentry-clj. Getting rid of raven lib. #65

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion deps.edn
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
{org.clojure/clojure "1.9.0"
org.clojure/tools.logging "0.4.0"
com.stuartsierra/component "0.3.2"
exoscale/raven "0.4.2"
io.sentry/sentry-clj "7.4.213"
spootnik/net "0.3.3-beta24"
spootnik/uncaught "0.5.3"
metrics-clojure "2.10.0"
Expand Down
2 changes: 1 addition & 1 deletion project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
[org.clojure/clojure "1.10.1"]
[org.clojure/tools.logging "1.0.0"]
[com.stuartsierra/component "1.0.0"]
[exoscale/raven "0.4.15"]
[io.sentry/sentry-clj "7.4.213"]
[spootnik/uncaught "0.5.5"]
[metrics-clojure "2.10.0"]
[metrics-clojure-riemann "2.10.0"]
Expand Down
10 changes: 4 additions & 6 deletions src/spootnik/reporter.clj
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
(ns spootnik.reporter
(:require [com.stuartsierra.component :as c]
[clojure.tools.logging :as log]
[raven.client :as raven]
[spootnik.reporter.impl :as rptr]
[manifold.deferred :as d]
spootnik.reporter.specs))
Expand Down Expand Up @@ -80,11 +79,10 @@

([error extra]
(log/error error)
(-> (capture! (-> (if (instance? Exception error)
(-> {:data (ex-data error)}
(raven/add-exception! error))
{:message error})
(raven/add-extra! extra)))
(-> (capture! (if (instance? Exception error)
{:extra extra
:throwable error}
{:message error}))
(d/catch Throwable
(fn [e]
(log/error e "Sentry failure")))
Expand Down
39 changes: 16 additions & 23 deletions src/spootnik/reporter/impl.clj
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
(ns spootnik.reporter.impl
(:require [aleph.http :as http]
[manifold.deferred :as d]
[clojure.java.io :as io]
[com.stuartsierra.component :as c]
[raven.client :as raven]
[metrics.reporters.console :as console]
[metrics.reporters.jmx :as jmx]
[metrics.reporters.graphite :as graphite]
Expand All @@ -19,7 +17,8 @@
[camel-snake-kebab.core :as csk]
[clojure.string :as str]
[clojure.tools.logging :refer [info error]]
[spootnik.uncaught :refer [with-uncaught]])
[spootnik.uncaught :refer [with-uncaught]]
[spootnik.reporter.sentry :as rs])
(:import com.aphyr.riemann.client.RiemannClient
com.aphyr.riemann.client.RiemannBatchClient
com.aphyr.riemann.client.TcpTransport
Expand Down Expand Up @@ -209,7 +208,7 @@
state (or (:state ev) (:state defaults))
time ^Long (or time (quot (System/currentTimeMillis) 1000))]
(-> (.event client)
(.host (or host (:host defaults) (raven/localhost)))
(.host (or host (:host defaults) (rs/localhost)))
(.service (or service (:service defaults) "<none>"))
(.time (long time))
(cond-> metric (.metric metric)
Expand Down Expand Up @@ -372,16 +371,15 @@
(defn parse-pggrouping-keys [grouping-keys]
(into {} (for [[k v] grouping-keys] [(csk/->snake_case_string k) v])))

;; "sentry" is a sentry map like {:dsn "..."}
;; "raven-options" is the options map sent to raven http client
;; http://aleph.io/codox/aleph/aleph.http.html#var-request
;; "sentry" is a configuration map sent to sentry.io on initialisation
Copy link
Contributor

@mping-exo mping-exo Mar 7, 2024

Choose a reason for hiding this comment

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

When migrating to the new reporter, we will have to provide the environment and release, instead of reading it off from SENTRY_* env vars right? Might be worth it to put it in the readme/changelog

Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't get this

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean that we may want to update the README to say clients should specifically pass environment and release when sending events.
Eg blockproxy was relying on SENTRY_ env vars: https://github.com/exoscale/blockstorage/blob/master/.kargo/collector.edn#L36 and io.sentry.logback.SentryAppender, which would read off those env vars; when we move to the new reporter, we have to explicitly pass these variables to the sentry config

;; https://github.com/getsentry/sentry-clj/tree/master?tab=readme-ov-file#additional-initialisation-options
(defrecord Reporter [rclient raven-options reporters registry sentry metrics riemann prevent-capture? prometheus
started? pushgateway]
c/Lifecycle
(start [this]
(if started?
this
(let [prometheus-registry (CollectorRegistry/defaultRegistry)
(let [prometheus-registry (CollectorRegistry/defaultRegistry)
[pgclient pgjob pgregistry pggrouping-keys] (when pushgateway [(build-pushgateway-client pushgateway)
(name (:job pushgateway))
(CollectorRegistry.)
Expand All @@ -405,6 +403,9 @@
prometheus
prometheus-registry)
opts)))]

(rs/init! (:dsn sentry) sentry)

(when-not prevent-capture?
(with-uncaught e
(capture! (assoc this :raven-options options) e)))
Expand Down Expand Up @@ -436,7 +437,10 @@
(.close ^RiemannClient rclient)
(catch Exception _)))
(when prometheus
(.close ^java.io.Closeable (:server prometheus))))
(.close ^java.io.Closeable (:server prometheus)))

(rs/close! (:dsn sentry)))

(assoc this
:raven-options nil
:reporters nil
Expand Down Expand Up @@ -516,21 +520,10 @@
SentrySink
(capture! [this e]
(capture! this e {}))
(capture! [this e tags]
(if (:dsn sentry)
(-> (try
(raven/capture! raven-options (:dsn sentry) e tags)
(catch Exception e
(d/error-deferred e)))
(d/chain
(fn [event-id]
(error e (str "captured exception as sentry event: " event-id))))
(d/catch (fn [e']
(error e "Failed to capture exception" {:tags tags :capture-exception e'})
(capture! this e'))))
(error e)))
(capture! [_this e tags]
(rs/send-event! (:dsn sentry) raven-options e tags))
RiemannSink
(send! [this ev]
(send! [_this ev]
(when rclient
(let [to-seq #(if-not (sequential? %) [%] %)]
(->> ev
Expand Down
98 changes: 98 additions & 0 deletions src/spootnik/reporter/sentry.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
(ns spootnik.reporter.sentry
(:require
[sentry-clj.core :as sentry-io]
[manifold.deferred :as d]
[clojure.string :as str]
[clojure.tools.logging :refer [error]]
[clojure.java.shell :as sh]))

(def http-requests-payload-stub
"Storage for stubbed http sentry events "
(atom nil))

(defn in-memory? [dsn]
(or (= dsn ":memory:")
(nil? dsn)))

(def hostname-refresh-interval
"How often to allow reading /etc/hostname, in seconds."
60)

(defn get-hostname
"Get the current hostname by shelling out to 'hostname'"
[]
(or
(try
(let [{:keys [exit out]} (sh/sh "hostname")]
(when (= exit 0)
(str/trim out)))
(catch Exception _))
"<unknown>"))

(defn hostname
"Fetches the hostname by shelling to 'hostname', whenever the given age
is stale enough. If the given age is recent, as defined by
hostname-refresh-interval, returns age and val instead."
[[age val]]
(if (and val (<= (* 1000 hostname-refresh-interval)
(- (System/currentTimeMillis) age)))
[age val]
[(System/currentTimeMillis) (get-hostname)]))

(let [cache (atom [nil nil])]
(defn localhost
"Returns the local host name."
[]
(if (re-find #"^Windows" (System/getProperty "os.name"))
(or (System/getenv "COMPUTERNAME") "localhost")
(or (System/getenv "HOSTNAME")
(second (swap! cache hostname))))))

(defn- e->sentry-request [e]
(let [payload (-> e :extra :payload)]
{:url (-> payload :uri)
:method (-> payload :request-method name clojure.string/upper-case)
:query-string (-> payload :query-string)
:headers (-> payload :headers)}))

(defn e->sentry-event [e options tags]
(let [{:keys [message extra throwable]} e]

(if message
arthuraliiev marked this conversation as resolved.
Show resolved Hide resolved
{:message message}

{:message (ex-message e)
:request (e->sentry-request e)
:level :error
:platform "java"
:user {:id (-> e :extra :org/uuid str)}
:tags tags
:server-name (localhost)
:fingerprints (some-> options :fingerpint seq)
:extra extra
:throwable throwable})))

(defn send-event! [dsn options e tags]
(let [event (e->sentry-event e options tags)]
(if-not (in-memory? dsn)
(-> (try
(sentry-io/send-event event)
(catch Exception e
(d/error-deferred e)))
(d/chain
arthuraliiev marked this conversation as resolved.
Show resolved Hide resolved
(fn [event-id]
(error e (str "captured exception as sentry event: " event-id))))
(d/catch (fn [e']
(error e "Failed to capture exception" {:tags tags :capture-exception e'})
(send-event! dsn options e' tags))))
(swap! http-requests-payload-stub conj event))))

(defn init! [dsn sentry]
(if-not (in-memory? dsn)
arthuraliiev marked this conversation as resolved.
Show resolved Hide resolved
(sentry-io/init! (:dsn sentry) sentry)
(reset! http-requests-payload-stub [])))

(defn close! [dsn]
(if-not (in-memory? dsn)
(sentry-io/close!)
(reset! http-requests-payload-stub nil)))
2 changes: 1 addition & 1 deletion test/spootnik/reporter/impl_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
[prometheus.core :as prometheus]
[spootnik.reporter.impl :refer :all]
[com.stuartsierra.component :as component]
[raven.client :refer [http-requests-payload-stub]]
[spootnik.reporter.sentry :refer [http-requests-payload-stub]]
[clojure.set :as cljset])
(:import io.prometheus.client.CollectorRegistry
io.netty.handler.ssl.SslContextBuilder
Expand Down
Loading