Skip to content

Add transportlogger util#40579

Open
smallinsky wants to merge 2 commits intomasterfrom
smallinsky/transport_logger_util
Open

Add transportlogger util#40579
smallinsky wants to merge 2 commits intomasterfrom
smallinsky/transport_logger_util

Conversation

@smallinsky
Copy link
Copy Markdown
Contributor

@smallinsky smallinsky commented Apr 16, 2024 •
edited
Loading

What

third party integrations are hard. In OKTA integration we encounter severals problems:

  • rate limiting
  • context cancelation
  • API failture
  • Unexpected Large number of calls cause by implementation bug

Add a http.Trasnport util allows to collect prometheus metrics for third party API integration like OKTA allowing to give a insight about external API calls

Related:
https://github.com/gravitational/teleport.e/pull/3970

Usage:

Many REST endpoints leverage URL entry where uuid is embedded in URL:
In order to minimalist URL to known set of values transport logger provide the

transportlogger.WithMetricInfo(ctx, transportlogger.MetricsInfo{CallName: "/api/v1/apps/:appID/users"})

method that allow to set the endpoint that will be reported in prometheus metrics:

A new context needs to be forwarded to http client via req.WithContext(ctx) or via library SDK library interface:

	ctx = transportlogger.WithMetricInfo(ctx, transportlogger.MetricsInfo{CallName: "/api/v1/apps/:appID/users"})
	appUsers, resp, err := w.client.Application.ListApplicationUsers(ctx, appID, query.NewQueryParams(
		query.WithLimit(500),
	))

The transport RoundTripper wrapper will collect prometheus metrics allowing to monitor third party API integration:

Screenshot 2024-04-16 at 16 45 05

@smallinsky smallinsky force-pushed the smallinsky/transport_logger_util branch 4 times, most recently from b28f4d9 to 280fe6d Compare April 16, 2024 15:07
@smallinsky smallinsky requested review from mdwn, r0mant and tcsc April 16, 2024 15:08
mdwn
mdwn reviewed Apr 16, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@mdwn mdwn left a comment

Choose a reason for hiding this comment

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

Before getting too far into this review, can https://pkg.go.dev/github.com/prometheus/client_golang/prometheus/promhttp be used instead?

api/utils/transportlogger/common/result.go Outdated
Copy link
Copy Markdown
Contributor

@mdwn mdwn Apr 16, 2024

Choose a reason for hiding this comment

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

nit

Suggested change
Copyright 2021 Gravitational, Inc.
Copyright 2024 Gravitational, Inc.

api/utils/transportlogger/logger.go Outdated
Copy link
Copy Markdown
Contributor

@mdwn mdwn Apr 16, 2024

Choose a reason for hiding this comment

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

Suggested change
Copyright 2021 Gravitational, Inc.
Copyright 2024 Gravitational, Inc.

api/utils/transportlogger/metrics_info.go Outdated
Copy link
Copy Markdown
Contributor

@mdwn mdwn Apr 16, 2024

Choose a reason for hiding this comment

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

Suggested change
Copyright 2021 Gravitational, Inc.
Copyright 2024 Gravitational, Inc.

api/utils/transportlogger/providers/prometheus/http_internals.go Outdated
Copy link
Copy Markdown
Contributor

@mdwn mdwn Apr 16, 2024

Choose a reason for hiding this comment

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

Suggested change
Copyright 2021 Gravitational, Inc.
Copyright 2024 Gravitational, Inc.

api/utils/transportlogger/providers/prometheus/metrics.go Outdated
Copy link
Copy Markdown
Contributor

@mdwn mdwn Apr 16, 2024

Choose a reason for hiding this comment

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

Suggested change
Copyright 2021 Gravitational, Inc.
Copyright 2024 Gravitational, Inc.

api/utils/transportlogger/providers/prometheus/prometheus.go Outdated
Copy link
Copy Markdown
Contributor

@mdwn mdwn Apr 16, 2024

Choose a reason for hiding this comment

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

Suggested change
Copyright 2021 Gravitational, Inc.
Copyright 2024 Gravitational, Inc.

api/utils/transportlogger/transport.go Outdated
Copy link
Copy Markdown
Contributor

@mdwn mdwn Apr 16, 2024

Choose a reason for hiding this comment

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

Suggested change
Copyright 2021 Gravitational, Inc.
Copyright 2024 Gravitational, Inc.

@smallinsky smallinsky force-pushed the smallinsky/transport_logger_util branch from 280fe6d to 6fea76d Compare April 16, 2024 18:51
@smallinsky
Copy link
Copy Markdown
Contributor Author

smallinsky commented Apr 16, 2024

Before getting too far into this review, can https://pkg.go.dev/github.com/prometheus/client_golang/prometheus/promhttp be used instead?

I don't think that promhttp allows to inject custom label per single client call.

@smallinsky smallinsky requested a review from mdwn April 18, 2024 10:15
@smallinsky smallinsky marked this pull request as ready for review April 18, 2024 10:15
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 18, 2024

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@github-actions github-actions bot requested a review from kimlisa April 18, 2024 10:16
r0mant
r0mant approved these changes Apr 22, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

lgtm

api/utils/transportlogger/providers/prometheus/metrics.go
ExternalAPICallMetric = prometheus.NewCounterVec(prometheus.CounterOpts{
Namespace: NamespaceTeleport,
Name: APICallStatusName,
Help: "Track calls to 3th party API",
Copy link
Copy Markdown
Collaborator

@r0mant r0mant Apr 22, 2024

Choose a reason for hiding this comment

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

Suggested change
Help: "Track calls to 3th party API",
Help: "Track calls to 3rd party API",

strideynet
strideynet reviewed May 8, 2024
View reviewed changes
api/utils/transportlogger/providers/prometheus/prometheus.go
return
}
ExternalAPICallMetric.With(prometheus.Labels{
endpointLabel: result.Name,
Copy link
Copy Markdown
Contributor

@strideynet strideynet May 8, 2024

Choose a reason for hiding this comment

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

Doesn't this risk being very high cardinality metric if IDs etc are included in the path of requests e.g GET /v1/foo/aa-bb-cc-dd ?

@smallinsky smallinsky mentioned this pull request Oct 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@strideynet strideynet strideynet left review comments

@r0mant r0mant r0mant approved these changes

@tcsc tcsc Awaiting requested review from tcsc

@kimlisa kimlisa Awaiting requested review from kimlisa

+1 more reviewer

@mdwn mdwn mdwn approved these changes

Reviewers whose approvals may not affect merge requirements

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

size/md

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@smallinsky @r0mant @mdwn @strideynet