From 9fbf1c5917a695cca0b1a32f8ab935a6e70573ae Mon Sep 17 00:00:00 2001 From: KAcper Perschke Date: Tue, 5 Nov 2024 20:03:37 +0100 Subject: [PATCH 1/4] Some refactor around license. Trying to put responsibility in the right places. --- artifactory/system.go | 19 ++++++++ collector/collector.go | 44 ++--------------- collector/system.go | 104 ++++++++++++++++++++++++++++++++--------- 3 files changed, 104 insertions(+), 63 deletions(-) diff --git a/artifactory/system.go b/artifactory/system.go index d3f5fb4..245a252 100644 --- a/artifactory/system.go +++ b/artifactory/system.go @@ -2,6 +2,8 @@ package artifactory import ( "encoding/json" + "slices" + "strings" ) const ( @@ -67,6 +69,23 @@ type LicenseInfo struct { ValidThrough string `json:"validThrough"` LicensedTo string `json:"licensedTo"` NodeId string + ValidSeconds int64 // It will be calculated in the ‘collector’ package. +} + +func (l LicenseInfo) NormalizedLicenseType() string { + return strings.ToLower(l.Type) +} + +func (l LicenseInfo) IsOSS() bool { + var afOSSLicenseTypes = []string{ + `community edition for c/c++`, + `jcr edition`, + `oss`, + } + return slices.Contains( + afOSSLicenseTypes, + l.NormalizedLicenseType(), + ) } // FetchLicense makes the API call to license endpoint and returns LicenseInfo diff --git a/collector/collector.go b/collector/collector.go index 9d55ee4..f8f075e 100755 --- a/collector/collector.go +++ b/collector/collector.go @@ -1,8 +1,6 @@ package collector import ( - "strings" - "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/version" ) @@ -134,52 +132,16 @@ func (e *Exporter) Collect(ch chan<- prometheus.Metric) { func (e *Exporter) scrape(ch chan<- prometheus.Metric) (up float64) { e.totalScrapes.Inc() - // Collect License info - var licenseType string - license, err := e.client.FetchLicense() - if err != nil { - e.totalAPIErrors.Inc() - return 0 - } - licenseType = strings.ToLower(license.Type) - // Some API endpoints are not available in OSS - if licenseType != "oss" && licenseType != "jcr edition" && licenseType != "community edition for c/c++" { - for metricName, metric := range securityMetrics { - switch metricName { - case "users": - err := e.exportUsersCount(metricName, metric, ch) - if err != nil { - return 0 - } - case "groups": - err := e.exportGroups(metricName, metric, ch) - if err != nil { - return 0 - } - case "certificates": - err := e.exportCertificates(metricName, metric, ch) - if err != nil { - return 0 - } - } - } - err = e.exportReplications(ch) - if err != nil { - return 0 - } - } - // Collect and export open metrics if e.optionalMetrics.OpenMetrics { - err = e.exportOpenMetrics(ch) + err := e.exportOpenMetrics(ch) if err != nil { return 0 } } // Collect and export system metrics - err = e.exportSystem(license, ch) - if err != nil { + if err := e.exportSystem(ch); err != nil { return 0 } @@ -200,7 +162,7 @@ func (e *Exporter) scrape(ch chan<- prometheus.Metric) (up float64) { // Get Downloaded and Created items for all repo in the last 1 and 5 minutes and add it to repoSummaryList if e.optionalMetrics.Artifacts { - repoSummaryList, err = e.getTotalArtifacts(repoSummaryList) + repoSummaryList, err := e.getTotalArtifacts(repoSummaryList) if err != nil { return 0 } diff --git a/collector/system.go b/collector/system.go index f72ce65..309b97d 100644 --- a/collector/system.go +++ b/collector/system.go @@ -1,7 +1,6 @@ package collector import ( - "strings" "time" "github.com/prometheus/client_golang/prometheus" @@ -9,8 +8,69 @@ import ( "github.com/peimanja/artifactory_exporter/artifactory" ) -func (e *Exporter) exportSystem(license artifactory.LicenseInfo, ch chan<- prometheus.Metric) error { - health, err := e.client.FetchHealth() +var afOSSLicenseTypes = []string{ + `oss`, + `jcr edition`, + `community edition for c/c++`, +} + +func collectLicense(e *Exporter, ch chan<- prometheus.Metric) (artifactory.LicenseInfo, error) { + retErr := func(err error) (artifactory.LicenseInfo, error) { + return artifactory.LicenseInfo{}, err + } + + license, err := e.client.FetchLicense() + if err != nil { + return retErr(err) + } + + if !license.IsOSS() { // Some endpoints are only available commercially. + for metricName, metric := range securityMetrics { + switch metricName { + case "users": + err := e.exportUsersCount(metricName, metric, ch) + if err != nil { + return retErr(err) + } + case "groups": + err := e.exportGroups(metricName, metric, ch) + if err != nil { + return retErr(err) + } + case "certificates": + err := e.exportCertificates(metricName, metric, ch) + if err != nil { + return retErr(err) + } + } + } + if err := e.exportReplications(ch); err != nil { + return retErr(err) + } + } + + licenseValidSeconds := func() int64 { + if license.IsOSS() { + return 0 + } + validThroughTime, err := time.Parse("Jan 2, 2006", license.ValidThrough) + if err != nil { + e.logger.Warn( + "Couldn't parse Artifactory license ValidThrough", + "err", err.Error(), + ) + return 0 // We deliberately ignore the error in order to maintain continuity. + } + validThroughEpoch := validThroughTime.Unix() + timeNowEpoch := time.Now().Unix() + return validThroughEpoch - timeNowEpoch + } + license.ValidSeconds = licenseValidSeconds() + return license, nil +} + +func (e *Exporter) exportSystem(ch chan<- prometheus.Metric) error { + healthInfo, err := e.client.FetchHealth() if err != nil { e.logger.Error( "Couldn't scrape Artifactory when fetching system/ping", @@ -28,32 +88,32 @@ func (e *Exporter) exportSystem(license artifactory.LicenseInfo, ch chan<- prome e.totalAPIErrors.Inc() return err } + licenseInfo, err := collectLicense(e, ch) + if err != nil { + e.logger.Error( + "Couldn't scrape Artifactory when fetching system/license", + "err", err.Error(), + ) + e.totalAPIErrors.Inc() + return err + } - licenseType := strings.ToLower(license.Type) for metricName, metric := range systemMetrics { switch metricName { case "healthy": - ch <- prometheus.MustNewConstMetric(metric, prometheus.GaugeValue, convArtiToPromBool(health.Healthy), health.NodeId) + ch <- prometheus.MustNewConstMetric(metric, prometheus.GaugeValue, convArtiToPromBool(healthInfo.Healthy), healthInfo.NodeId) case "version": ch <- prometheus.MustNewConstMetric(metric, prometheus.GaugeValue, 1, buildInfo.Version, buildInfo.Revision, buildInfo.NodeId) case "license": - var validThrough float64 - timeNow := float64(time.Now().Unix()) - switch licenseType { - case "oss", "jcr edition", "community edition for c/c++": - validThrough = timeNow - default: - if validThroughTime, err := time.Parse("Jan 2, 2006", license.ValidThrough); err != nil { - e.logger.Warn( - "Couldn't parse Artifactory license ValidThrough", - "err", err.Error(), - ) - validThrough = timeNow - } else { - validThrough = float64(validThroughTime.Unix()) - } - } - ch <- prometheus.MustNewConstMetric(metric, prometheus.GaugeValue, validThrough-timeNow, licenseType, license.LicensedTo, license.ValidThrough, license.NodeId) + ch <- prometheus.MustNewConstMetric( + metric, + prometheus.GaugeValue, + float64(licenseInfo.ValidSeconds), //float + licenseInfo.NormalizedLicenseType(), + licenseInfo.LicensedTo, + licenseInfo.ValidThrough, + licenseInfo.NodeId, + ) } } return nil From 839c38a25389c16ba9bedb796e8b77dd10cf3167 Mon Sep 17 00:00:00 2001 From: KAcper Perschke Date: Tue, 5 Nov 2024 20:10:40 +0100 Subject: [PATCH 2/4] Minor amendments. Shortcomings left over from rapid development. --- artifactory/system.go | 4 ++-- collector/system.go | 10 ++-------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/artifactory/system.go b/artifactory/system.go index 245a252..d47ae41 100644 --- a/artifactory/system.go +++ b/artifactory/system.go @@ -72,7 +72,7 @@ type LicenseInfo struct { ValidSeconds int64 // It will be calculated in the ‘collector’ package. } -func (l LicenseInfo) NormalizedLicenseType() string { +func (l LicenseInfo) TypeNormalized() string { return strings.ToLower(l.Type) } @@ -84,7 +84,7 @@ func (l LicenseInfo) IsOSS() bool { } return slices.Contains( afOSSLicenseTypes, - l.NormalizedLicenseType(), + l.TypeNormalized(), ) } diff --git a/collector/system.go b/collector/system.go index 309b97d..f656bc2 100644 --- a/collector/system.go +++ b/collector/system.go @@ -8,12 +8,6 @@ import ( "github.com/peimanja/artifactory_exporter/artifactory" ) -var afOSSLicenseTypes = []string{ - `oss`, - `jcr edition`, - `community edition for c/c++`, -} - func collectLicense(e *Exporter, ch chan<- prometheus.Metric) (artifactory.LicenseInfo, error) { retErr := func(err error) (artifactory.LicenseInfo, error) { return artifactory.LicenseInfo{}, err @@ -108,8 +102,8 @@ func (e *Exporter) exportSystem(ch chan<- prometheus.Metric) error { ch <- prometheus.MustNewConstMetric( metric, prometheus.GaugeValue, - float64(licenseInfo.ValidSeconds), //float - licenseInfo.NormalizedLicenseType(), + float64(licenseInfo.ValidSeconds), // Prometheus expects a float type. + licenseInfo.TypeNormalized(), licenseInfo.LicensedTo, licenseInfo.ValidThrough, licenseInfo.NodeId, From d200566355cf598de331375438a37aaf8909d923 Mon Sep 17 00:00:00 2001 From: KAcper Perschke Date: Tue, 5 Nov 2024 20:54:21 +0100 Subject: [PATCH 3/4] Sonnar issue Cognitive Complexity. --- artifactory/system.go | 27 ++++++++++--- collector/security.go | 26 ++++++++++++ collector/system.go | 93 ++++++++++++++----------------------------- 3 files changed, 78 insertions(+), 68 deletions(-) diff --git a/artifactory/system.go b/artifactory/system.go index d47ae41..b093c21 100644 --- a/artifactory/system.go +++ b/artifactory/system.go @@ -2,8 +2,10 @@ package artifactory import ( "encoding/json" + "fmt" "slices" "strings" + "time" ) const ( @@ -69,11 +71,6 @@ type LicenseInfo struct { ValidThrough string `json:"validThrough"` LicensedTo string `json:"licensedTo"` NodeId string - ValidSeconds int64 // It will be calculated in the ‘collector’ package. -} - -func (l LicenseInfo) TypeNormalized() string { - return strings.ToLower(l.Type) } func (l LicenseInfo) IsOSS() bool { @@ -88,6 +85,26 @@ func (l LicenseInfo) IsOSS() bool { ) } +func (l LicenseInfo) TypeNormalized() string { + return strings.ToLower(l.Type) +} + +func (l LicenseInfo) ValidSeconds() (int64, error) { + if l.IsOSS() { + return 0, nil + } + validThroughTime, err := time.Parse("Jan 2, 2006", l.ValidThrough) + if err != nil { + return 0, fmt.Errorf( + "unparsable ‘validThrough’ license field: %w", + err, + ) + } + validThroughEpoch := validThroughTime.Unix() + timeNowEpoch := time.Now().Unix() + return validThroughEpoch - timeNowEpoch, nil +} + // FetchLicense makes the API call to license endpoint and returns LicenseInfo func (c *Client) FetchLicense() (LicenseInfo, error) { var licenseInfo LicenseInfo diff --git a/collector/security.go b/collector/security.go index efa86a8..f50e871 100644 --- a/collector/security.go +++ b/collector/security.go @@ -26,6 +26,32 @@ func (e *Exporter) countUsersPerRealm(users []artifactory.User) realmUserCounts return usersPerRealm } +func (e *Exporter) exportAllSecurityMetrics(ch chan<- prometheus.Metric) error { + for metricName, metric := range securityMetrics { + switch metricName { + case "users": + err := e.exportUsersCount(metricName, metric, ch) + if err != nil { + return err + } + case "groups": + err := e.exportGroups(metricName, metric, ch) + if err != nil { + return err + } + case "certificates": + err := e.exportCertificates(metricName, metric, ch) + if err != nil { + return err + } + } + } + if err := e.exportReplications(ch); err != nil { + return err + } + return nil +} + func (e *Exporter) exportUsersCount(metricName string, metric *prometheus.Desc, ch chan<- prometheus.Metric) error { // Fetch Artifactory Users users, err := e.client.FetchUsers() diff --git a/collector/system.go b/collector/system.go index f656bc2..33c1c2c 100644 --- a/collector/system.go +++ b/collector/system.go @@ -1,68 +1,9 @@ package collector import ( - "time" - "github.com/prometheus/client_golang/prometheus" - - "github.com/peimanja/artifactory_exporter/artifactory" ) -func collectLicense(e *Exporter, ch chan<- prometheus.Metric) (artifactory.LicenseInfo, error) { - retErr := func(err error) (artifactory.LicenseInfo, error) { - return artifactory.LicenseInfo{}, err - } - - license, err := e.client.FetchLicense() - if err != nil { - return retErr(err) - } - - if !license.IsOSS() { // Some endpoints are only available commercially. - for metricName, metric := range securityMetrics { - switch metricName { - case "users": - err := e.exportUsersCount(metricName, metric, ch) - if err != nil { - return retErr(err) - } - case "groups": - err := e.exportGroups(metricName, metric, ch) - if err != nil { - return retErr(err) - } - case "certificates": - err := e.exportCertificates(metricName, metric, ch) - if err != nil { - return retErr(err) - } - } - } - if err := e.exportReplications(ch); err != nil { - return retErr(err) - } - } - - licenseValidSeconds := func() int64 { - if license.IsOSS() { - return 0 - } - validThroughTime, err := time.Parse("Jan 2, 2006", license.ValidThrough) - if err != nil { - e.logger.Warn( - "Couldn't parse Artifactory license ValidThrough", - "err", err.Error(), - ) - return 0 // We deliberately ignore the error in order to maintain continuity. - } - validThroughEpoch := validThroughTime.Unix() - timeNowEpoch := time.Now().Unix() - return validThroughEpoch - timeNowEpoch - } - license.ValidSeconds = licenseValidSeconds() - return license, nil -} - func (e *Exporter) exportSystem(ch chan<- prometheus.Metric) error { healthInfo, err := e.client.FetchHealth() if err != nil { @@ -82,7 +23,7 @@ func (e *Exporter) exportSystem(ch chan<- prometheus.Metric) error { e.totalAPIErrors.Inc() return err } - licenseInfo, err := collectLicense(e, ch) + licenseInfo, err := e.client.FetchLicense() if err != nil { e.logger.Error( "Couldn't scrape Artifactory when fetching system/license", @@ -91,18 +32,37 @@ func (e *Exporter) exportSystem(ch chan<- prometheus.Metric) error { e.totalAPIErrors.Inc() return err } + licenseValSec, err := licenseInfo.ValidSeconds() + if err != nil { + e.logger.Warn( + "Couldn't get Artifactory license validity", + "err", err.Error(), + ) // To preserve the operation, we do nothing but log the event, + } for metricName, metric := range systemMetrics { switch metricName { case "healthy": - ch <- prometheus.MustNewConstMetric(metric, prometheus.GaugeValue, convArtiToPromBool(healthInfo.Healthy), healthInfo.NodeId) + ch <- prometheus.MustNewConstMetric( + metric, + prometheus.GaugeValue, + convArtiToPromBool(healthInfo.Healthy), + healthInfo.NodeId, + ) case "version": - ch <- prometheus.MustNewConstMetric(metric, prometheus.GaugeValue, 1, buildInfo.Version, buildInfo.Revision, buildInfo.NodeId) + ch <- prometheus.MustNewConstMetric( + metric, + prometheus.GaugeValue, + 1, + buildInfo.Version, + buildInfo.Revision, + buildInfo.NodeId, + ) case "license": ch <- prometheus.MustNewConstMetric( metric, prometheus.GaugeValue, - float64(licenseInfo.ValidSeconds), // Prometheus expects a float type. + float64(licenseValSec), // Prometheus expects a float type. licenseInfo.TypeNormalized(), licenseInfo.LicensedTo, licenseInfo.ValidThrough, @@ -110,5 +70,12 @@ func (e *Exporter) exportSystem(ch chan<- prometheus.Metric) error { ) } } + if !licenseInfo.IsOSS() { // Some endpoints are only available commercially. + err := e.exportAllSecurityMetrics(ch) + if err != nil { + return err + } + } + return nil } From 1eae56c26602f26e4da2a64ed1b2db779c9aceb1 Mon Sep 17 00:00:00 2001 From: KAcper Perschke Date: Tue, 5 Nov 2024 21:03:47 +0100 Subject: [PATCH 4/4] Time format. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To quote Oracle “Month name-Day-Year with no leading zeros”. --- artifactory/system.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/artifactory/system.go b/artifactory/system.go index b093c21..f21aa4c 100644 --- a/artifactory/system.go +++ b/artifactory/system.go @@ -89,11 +89,13 @@ func (l LicenseInfo) TypeNormalized() string { return strings.ToLower(l.Type) } +const USAFullDate = "Jan 2, 2006" + func (l LicenseInfo) ValidSeconds() (int64, error) { if l.IsOSS() { return 0, nil } - validThroughTime, err := time.Parse("Jan 2, 2006", l.ValidThrough) + validThroughTime, err := time.Parse(USAFullDate, l.ValidThrough) if err != nil { return 0, fmt.Errorf( "unparsable ‘validThrough’ license field: %w",