From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 100450 invoked by alias); 15 Nov 2019 17:26:13 -0000 Mailing-List: contact elfutils-devel-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Post: List-Help: List-Subscribe: Sender: elfutils-devel-owner@sourceware.org Received: (qmail 100439 invoked by uid 89); 15 Nov 2019 17:26:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.3 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-19.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy=authorization, statistics, trusts X-Spam-Status: No, score=-19.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on sourceware.org X-Spam-Level: X-HELO: gnu.wildebeest.org Received: from wildebeest.demon.nl (HELO gnu.wildebeest.org) (212.238.236.112) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 15 Nov 2019 17:26:11 +0000 Received: from tarox.wildebeest.org (tarox.wildebeest.org [172.31.17.39]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id E9AFE3000275; Fri, 15 Nov 2019 18:26:06 +0100 (CET) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id 9C9594000FF8; Fri, 15 Nov 2019 18:26:06 +0100 (CET) Message-ID: Subject: Re: patch 5 debuginfod: prometheus metrics From: Mark Wielaard To: "Frank Ch. Eigler" , elfutils-devel@sourceware.org, amerey@redhat.com Date: Fri, 15 Nov 2019 17:26:00 -0000 In-Reply-To: <20191107090833.GB19337@redhat.com> References: <20191028190438.GC14349@redhat.com> <20191028190602.GD14349@redhat.com> <20191028190726.GE14349@redhat.com> <20191104214823.GA17633@redhat.com> <20191107090732.GA19337@redhat.com> <20191107090833.GB19337@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.28.5 (3.28.5-5.el7) Mime-Version: 1.0 X-Spam-Flag: NO X-IsSubscribed: yes X-SW-Source: 2019-q4/txt/msg00144.txt.bz2 Hi, On Thu, 2019-11-07 at 04:08 -0500, Frank Ch. Eigler wrote: > This webapi extensions allows admins to hook up debuginfod to a > prometheus-compatible monitoring system for general situational > statistics. The metrics are simple enough that local curl requests > can give a user a sense of what's going on. The metrics are > documented as unstable with respect to future versions. > +.SS /metrics > + > +This endpoint returns a Prometheus formatted text/plain dump of a > +variety of statistics about the operation of the debuginfod server. > +The exact set of metrics and their meanings may change in future > +versions. Caution: configuration information (path names, versions) > +may be disclosed. Could you also add a reference to the Prometheus Exposition format. I see it is already in a comment in the code. Best to also add it as See also in the docs. > .SH DATA MANAGEMENT >=20=20 > debuginfod stores its index in an sqlite database in a densely > packed > @@ -291,7 +299,8 @@ a denial-of-service in terms of RAM, CPU, disk > I/O, or network I/O. > If this is a problem, users are advised to install debuginfod with a > HTTPS reverse-proxy front-end that enforces site policies for > firewalling, authentication, integrity, authorization, and load > -control. > +control. The \fI/metrics\fP webapi endpoint is probably not > +appropriate for disclosure to the public. So, should there be an option to turn it off? =20 > When relaying queries to upstream debuginfods, debuginfod \fBdoes not\fP > include any particular security features. It trusts that the binaries > diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx > index 7c7a0c5d7ef5..efe8c80fa081 100644 > --- a/debuginfod/debuginfod.cxx > +++ b/debuginfod/debuginfod.cxx > @@ -72,6 +72,7 @@ extern "C" { > #include > #include > #include > +#include > #include > #include > #include > @@ -98,6 +99,14 @@ using namespace std; > #include > #endif >=20=20 > +#ifdef __linux__ > +#define gettid() syscall(SYS_gettid) > +#else > +#define gettid() pthread_self() > +#endif You might want to rename this since newer glibc might expose gettid(). The rest of the code looks good as far as I can see. But I would suggest you add a command line option to disable the metrics, which would not install the metrics handler and make the metrics update functions noops. > diff --git a/tests/ChangeLog b/tests/ChangeLog > index 3d50ee8623ee..156a693f8886 100644 > --- a/tests/ChangeLog > +++ b/tests/ChangeLog > @@ -1,3 +1,8 @@ > +2019-11-07 Frank Ch. Eigler > + > + * run-debuginfod-find.sh: Test debuginfod metrics via curl. > + Fix federated testing, asserted by metrics. > + > 2019-11-06 Frank Ch. Eigler >=20=20 > * run-debuginfod-find.sh: Test debuginfod -L mode. Drop > diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh > index 644901073d75..4c3e3cb306c2 100755 > --- a/tests/run-debuginfod-find.sh > +++ b/tests/run-debuginfod-find.sh > @@ -181,7 +181,8 @@ sleep 3 >=20=20 > # have clients contact the new server > export DEBUGINFOD_URLS=3Dhttp://localhost:$PORT2 > -testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog 1 > +rm -rf $DEBUGINFOD_CACHE_PATH > +testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID >=20=20 > # confirm that first server can't resolve symlinked info in L/ but secon= d can > BUILDID=3D`env LD_LIBRARY_PATH=3D$ldpath ${abs_builddir}/../src/readelf \ > @@ -202,6 +203,16 @@ export DEBUGINFOD_URLS=3D"BAD http://localhost:$PORT= 1 localhost:$PORT1 http://loca >=20=20 > testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog2 1 >=20=20 > +######################################################################## > + > +# Fetch some metrics, if curl program is installed > +if which curl 2>/dev/null; then > + curl http://localhost:$PORT1/badapi > + curl http://localhost:$PORT1/metrics > + curl http://localhost:$PORT2/metrics > + curl http://localhost:$PORT1/metrics | grep -q 'http_responses_total= .*result.*error' > + curl http://localhost:$PORT2/metrics | grep -q 'http_responses_total= .*result.*upstream' > +fi >=20=20 > ######################################################################## I think it is better to check with: if type curl >/dev/null 2>&1; then Which avoid executing which, which might not be installed... Cheers, Mark