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

Add metric for tmax timeout exceed count #2892

Merged
merged 7 commits into from
Aug 3, 2023
Merged

Conversation

Sonali-More-Xandr
Copy link
Contributor

@Sonali-More-Xandr Sonali-More-Xandr commented Jul 4, 2023

We have added Tmax.BidderResponseDurationMin check in auction workflow. As per this change, PBS won't send request to bidder server if remaining duration less than BidderResponseDurationMin. This PR adds a counter metric for such rejected request.

- As of now, `bidderRequestStartTime` is part of `ExtraRequestInfo`. The ExtraRequestInfo is further passed as details to Adapter's MakeRequest() implementation.

- We don't want to pass bidderRequestStartTime details to `MakeRequest()`. So remove bidderRequestStartTime from ExtraRequestInfo and add it in bidRequestOptions.
@onkarvhanumante onkarvhanumante self-assigned this Jul 4, 2023
@@ -197,6 +198,10 @@ func NewMetrics(cfg config.PrometheusMetrics, disabledMetrics config.DisabledMet
"connections_opened",
"Count of successful connections opened to Prebid Server.")

metrics.tmaxTimeout = newCounterWithoutLabels(cfg, reg,
"tmax_timeout",
"Count of requests rejected due to timeout exceed.")
Copy link
Contributor

Choose a reason for hiding this comment

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

lets add tmax in description. How about Count of requests rejected due to Tmax timeout exceed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated description in 62c4b50

@onkarvhanumante onkarvhanumante force-pushed the tmax/add-bidder-resp-min-check branch from ec51657 to 1b6ebea Compare July 25, 2023 07:14
@onkarvhanumante onkarvhanumante changed the base branch from tmax/add-bidder-resp-min-check to master July 31, 2023 09:36
@guscarreon
Copy link
Contributor

Hi @Sonali-More-Xandr . This pull request has been unblocked but there are conflicts that must be resolved.

@hhhjort hhhjort self-requested a review August 2, 2023 17:07
@hhhjort hhhjort self-assigned this Aug 2, 2023
Copy link
Collaborator

@hhhjort hhhjort left a comment

Choose a reason for hiding this comment

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

LGTM

@Sonali-More-Xandr Sonali-More-Xandr merged commit e637d0c into master Aug 3, 2023
@Sonali-More-Xandr Sonali-More-Xandr deleted the PBS-1173 branch August 3, 2023 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants