Add metrics #23
Add metrics #23
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds new metrics tracking capabilities to the invoker system while refactoring various pipeline components to use updated job fields and a more robust resource loading approach. Key changes include updating job identification to use the nested Job struct, integrating metrics collection with timings for key operations, and refactoring storage cache getters to use new naming.
Reviewed Changes
Copilot reviewed 23 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| master/queue/jobgenerators/icpc_generator.go | Update job ID accesses from result.JobID to result.Job.ID |
| master/invoker_handlers.go | Adjust logging and metrics submission to use the updated job ID field |
| lib/customfields/time.go & memory.go | Change GORM method signatures to the new style |
| invoker/* | Refactor various pipeline functions to use new resource loading methods and internal metrics |
| invoker/storage/*.go | Rename cacheGetter to CacheGetter for consistency |
| common/metrics/metrics.go | Introduce new Prometheus metrics collectors and counters |
| common/connectors/masterconn/structs.go | Update InvokerJobResult to embed Job and include attached metrics |
Files not reviewed (3)
- data/compile/config.yaml: Language not supported
- data/compile/scripts/g++.sh.tmpl: Language not supported
- go.mod: Language not supported
Comments suppressed due to low confidence (1)
common/metrics/metrics.go:37
- It appears 'sandbox_occupation_duration_sun' may be a typo and should likely be 'sandbox_occupation_duration_sum'.
InvokerSandboxOccupationDuration = createInvokerCounter("sandbox_occupation_duration_sun", "Total sandbox time for submission testing in invoker")
| var submission models.Submission | ||
| if err := i.TS.DB.WithContext(c).Find(&submission, job.SubmitID).Error; err != nil { | ||
| if errors.Is(err, gorm.ErrRecordNotFound) { | ||
| connector.RespErr(c, http.StatusBadRequest, "Submission %d not found", job.SubmitID) |
There was a problem hiding this comment.
Тут как бы мы говорим что джоба плохо построена, раз мы хотим чтобы она тестиировала посылку или задачу, которой нет. То есть 404 это логичная ошибка, когда мы хотим конкретный ресурс которого нет. А тут мы хотим сложное действие выполнить (джобу), которое некорректно задано
There was a problem hiding this comment.
Pull Request Overview
This PR adds new metrics for the invoker component and refactors several pipeline and storage access methods to integrate metric measurements.
- Introduces Prometheus metrics in common/metrics and updates invoker pipelines to record durations for file actions, execution, and resource waits.
- Refactors job state handling by renaming fields in Job and updating related resource access functions.
- Updates cache accessor types and logging functionalities to support these new metrics.
Reviewed Changes
Copilot reviewed 25 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| master/invoker_handlers.go | Updated job result handling to pass the new job structure to metrics. |
| lib/logger/logger.go | Modified logger creation API and added a Println method. |
| lib/customfields/time.go & memory.go | Renamed Gorm data type functions to GormDBDataType for improved DB support. |
| invoker/* | Updated pipeline state management, job initialization, and resource locks. |
| common/metrics/metrics.go | Introduced new Prometheus metrics for various invoker durations. |
| common/connectors/masterconn/structs.go | Changed JobID field to a Job pointer to include complete job details. |
Files not reviewed (3)
- data/compile/config.yaml: Language not supported
- data/compile/scripts/g++.sh.tmpl: Language not supported
- go.mod: Language not supported
Comments suppressed due to low confidence (1)
invoker/pipeline.go:91
- [nitpick] The function name 'checkFinish' may be misleading since it panics if the pipeline is not marked as finished. A more descriptive name (e.g., 'ensurePipelineFinished') could improve clarity.
func (s *JobPipelineState) checkFinish() {
Add metrics for invoker, more metrics will come in future