From c232dc9a52899f992765e8d5e77694b14adf711d Mon Sep 17 00:00:00 2001
From: Igor Drozdov <idrozdov@gitlab.com>
Date: Wed, 18 May 2022 17:39:52 +0400
Subject: Exclude API errors from error rate

When API isn't responsible or the resource is not accessible
(returns 404 or 403), then we shouldn't consider it as an error
on gitlab-sshd side
---
 client/gitlabnet.go              | 14 +++++++++++---
 internal/sshd/connection.go      |  7 ++++++-
 internal/sshd/connection_test.go | 11 +++++++++++
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/client/gitlabnet.go b/client/gitlabnet.go
index ac056aa..31131d2 100644
--- a/client/gitlabnet.go
+++ b/client/gitlabnet.go
@@ -37,6 +37,14 @@ type GitlabNetClient struct {
 	userAgent  string
 }
 
+type ApiError struct {
+	Msg string
+}
+
+func (e *ApiError) Error() string {
+	return e.Msg
+}
+
 func NewGitlabNetClient(
 	user,
 	password,
@@ -101,9 +109,9 @@ func parseError(resp *http.Response) error {
 	parsedResponse := &ErrorResponse{}
 
 	if err := json.NewDecoder(resp.Body).Decode(parsedResponse); err != nil {
-		return fmt.Errorf("Internal API error (%v)", resp.StatusCode)
+		return &ApiError{fmt.Sprintf("Internal API error (%v)", resp.StatusCode)}
 	} else {
-		return fmt.Errorf(parsedResponse.Message)
+		return &ApiError{parsedResponse.Message}
 	}
 
 }
@@ -157,7 +165,7 @@ func (c *GitlabNetClient) DoRequest(ctx context.Context, method, path string, da
 
 	if err != nil {
 		logger.WithError(err).Error("Internal API unreachable")
-		return nil, fmt.Errorf("Internal API unreachable")
+		return nil, &ApiError{"Internal API unreachable"}
 	}
 
 	if response != nil {
diff --git a/internal/sshd/connection.go b/internal/sshd/connection.go
index 0295d8f..d38e3d8 100644
--- a/internal/sshd/connection.go
+++ b/internal/sshd/connection.go
@@ -2,6 +2,7 @@ package sshd
 
 import (
 	"context"
+	"errors"
 	"time"
 
 	"golang.org/x/crypto/ssh"
@@ -9,6 +10,7 @@ import (
 	grpccodes "google.golang.org/grpc/codes"
 	grpcstatus "google.golang.org/grpc/status"
 
+	"gitlab.com/gitlab-org/gitlab-shell/client"
 	"gitlab.com/gitlab-org/gitlab-shell/internal/config"
 	"gitlab.com/gitlab-org/gitlab-shell/internal/metrics"
 
@@ -90,7 +92,10 @@ func (c *connection) handle(ctx context.Context, chans <-chan ssh.NewChannel, ha
 				if grpcstatus.Convert(err).Code() == grpccodes.Canceled {
 					metrics.SshdCanceledSessions.Inc()
 				} else {
-					metrics.SliSshdSessionsErrorsTotal.Inc()
+					var apiError *client.ApiError
+					if !errors.As(err, &apiError) {
+						metrics.SliSshdSessionsErrorsTotal.Inc()
+					}
 				}
 			}
 
diff --git a/internal/sshd/connection_test.go b/internal/sshd/connection_test.go
index f792300..fe7e4be 100644
--- a/internal/sshd/connection_test.go
+++ b/internal/sshd/connection_test.go
@@ -13,6 +13,7 @@ import (
 	grpccodes "google.golang.org/grpc/codes"
 	grpcstatus "google.golang.org/grpc/status"
 
+	"gitlab.com/gitlab-org/gitlab-shell/client"
 	"gitlab.com/gitlab-org/gitlab-shell/internal/config"
 	"gitlab.com/gitlab-org/gitlab-shell/internal/metrics"
 )
@@ -222,4 +223,14 @@ func TestSessionsMetrics(t *testing.T) {
 	require.InDelta(t, initialSessionsTotal+2, testutil.ToFloat64(metrics.SliSshdSessionsTotal), 0.1)
 	require.InDelta(t, initialSessionsErrorTotal+1, testutil.ToFloat64(metrics.SliSshdSessionsErrorsTotal), 0.1)
 	require.InDelta(t, initialCanceledSessions+1, testutil.ToFloat64(metrics.SshdCanceledSessions), 0.1)
+
+	conn, chans = setup(1, newChannel)
+	conn.handle(context.Background(), chans, func(context.Context, ssh.Channel, <-chan *ssh.Request) error {
+		close(chans)
+		return &client.ApiError{"api error"}
+	})
+
+	require.InDelta(t, initialSessionsTotal+3, testutil.ToFloat64(metrics.SliSshdSessionsTotal), 0.1)
+	require.InDelta(t, initialSessionsErrorTotal+1, testutil.ToFloat64(metrics.SliSshdSessionsErrorsTotal), 0.1)
+	require.InDelta(t, initialCanceledSessions+1, testutil.ToFloat64(metrics.SshdCanceledSessions), 0.1)
 }
-- 
cgit v1.2.1