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

[ENG-2342] ai_first_segment_delay metric added #3341

Merged

Conversation

pwilczynskiclearcode
Copy link
Contributor

Adding ai_first_segment_delay prometheus metric.
It measures the time between AI Gateway receiving a GenLiveVideoToVideo request and successfully receiving the first processed segment from O.

@github-actions github-actions bot added go Pull requests that update Go code AI Issues and PR related to the AI-video branch. labels Jan 7, 2025
@pwilczynskiclearcode pwilczynskiclearcode force-pushed the pawel/eng-2342-add-stream-startup-time-metric branch from 139f2c8 to c8cbe79 Compare January 7, 2025 15:33
Copy link

codecov bot commented Jan 7, 2025

Codecov Report

Attention: Patch coverage is 36.84211% with 24 lines in your changes missing coverage. Please review.

Project coverage is 33.72276%. Comparing base (17832e7) to head (ef06672).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
monitor/census.go 53.84615% 11 Missing and 1 partial ⚠️
server/ai_live_video.go 0.00000% 7 Missing ⚠️
server/ai_process.go 0.00000% 5 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #3341         +/-   ##
===================================================
- Coverage   33.72469%   33.72276%   -0.00193%     
===================================================
  Files            141         141                 
  Lines          37391       37405         +14     
===================================================
+ Hits           12610       12614          +4     
- Misses         24061       24070          +9     
- Partials         720         721          +1     
Files with missing lines Coverage Δ
server/ai_process.go 0.59880% <0.00000%> (-0.00206%) ⬇️
server/ai_live_video.go 0.00000% <0.00000%> (ø)
monitor/census.go 61.98675% <53.84615%> (+0.16736%) ⬆️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 17832e7...ef06672. Read the comment docs.

Files with missing lines Coverage Δ
server/ai_process.go 0.59880% <0.00000%> (-0.00206%) ⬇️
server/ai_live_video.go 0.00000% <0.00000%> (ø)
monitor/census.go 61.98675% <53.84615%> (+0.16736%) ⬆️

... and 1 file with indirect coverage changes

monitor/census.go Outdated Show resolved Hide resolved
server/ai_live_video.go Outdated Show resolved Hide resolved
@@ -1987,6 +1996,10 @@ func AIResultDownloaded(ctx context.Context, pipeline string, model string, down
}
}

func AIFirstSegmentDelay(delay int64) {
stats.Record(census.ctx, census.mAIFirstSegmentDelay.M(delay))
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we may want to tag it with Orchestrator's address?

I can imagine that for the given Gateway, only some Os may be slow.

Copy link
Contributor Author

@pwilczynskiclearcode pwilczynskiclearcode Jan 8, 2025

Choose a reason for hiding this comment

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

Yes. That would be interesting information.

The question is how to get Orchestrator info in startTrickleSubscribe() function.
Where of if does it live on the Gateway code? We'd need OrchestratorInfo struct, I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can pass host to startTrickleSubscribe().

This host is good as a tag. It's the IP/DNS of the Orchestrator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. I took entire orchInfo and unified it a bit on the census-side

@pwilczynskiclearcode pwilczynskiclearcode force-pushed the pawel/eng-2342-add-stream-startup-time-metric branch from 3b333a6 to de6fb68 Compare January 8, 2025 09:25
@pwilczynskiclearcode pwilczynskiclearcode force-pushed the pawel/eng-2342-add-stream-startup-time-metric branch from de6fb68 to 6a33ba2 Compare January 8, 2025 10:08
Copy link
Contributor

@leszko leszko left a comment

Choose a reason for hiding this comment

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

Added one more comment on how to tag with host

@pwilczynskiclearcode pwilczynskiclearcode force-pushed the pawel/eng-2342-add-stream-startup-time-metric branch from 6a33ba2 to 33cf5a6 Compare January 8, 2025 11:25
@pwilczynskiclearcode pwilczynskiclearcode force-pushed the pawel/eng-2342-add-stream-startup-time-metric branch from 33cf5a6 to ee71e3b Compare January 8, 2025 11:30
@pwilczynskiclearcode pwilczynskiclearcode force-pushed the pawel/eng-2342-add-stream-startup-time-metric branch from eeaf6f8 to 9e290c1 Compare January 8, 2025 11:57
@pwilczynskiclearcode pwilczynskiclearcode force-pushed the pawel/eng-2342-add-stream-startup-time-metric branch from 9e290c1 to ef06672 Compare January 8, 2025 12:10
@pwilczynskiclearcode pwilczynskiclearcode merged commit 8d6e7ec into master Jan 8, 2025
17 of 18 checks passed
@pwilczynskiclearcode pwilczynskiclearcode deleted the pawel/eng-2342-add-stream-startup-time-metric branch January 8, 2025 13:52
@livepeer livepeer deleted a comment from linear bot Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI Issues and PR related to the AI-video branch. go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants