feat: Add deal making trace

[bgins]

Summary

This pull request makes the following changes:

• Add solve trace
  • Add get_matching_deals trace
    • Add get_targeted_deal trace
    • Add match trace
  • Add add_deal trace
• Refactor match algorithm to make moduleID available one scope out

This pull request adds a trace to deal making in the solver. It observes matching and reports deals that were made and errors where a deal could not be made.

Preview image:

Task/Issue reference

Closes: #331

Test plan

Tracing over deal making includes a few spans:

• solve. The outermost parent span traces over the solve function.
• get_matching_deals. Child of the solve trace.
• get_targeted_deal. Child of get_matching_deals.
• match. Child of get_matching_deals.

Query for the solve span with:

{resource.service.name="solver" && name="solve"}

Add span.deal_ids != "[]" to filter out spans that did not match any deals:

{resource.service.name="solver" && name="solve" && span.deal_ids != "[]"}

Adding this filter will helps with testing because solve runs every ten seconds, but only one trace will make a match for a test job run.

The get_matching_deals span will appear as a child span. It can also be queried directly:

{resource.service.name="solver" && name="get_matching_deals" && span.job_offers != "[]"}

Filtering with span.job_offers != "[]" ignores spans without any matches because they lack a job offer to match on.

A get_matching_deals may contain one or more match spans. Each match span traces a match attempt between a job offer and a resource offer.

When using node targeting, a get_targeted_deal span will appear as a child of get_matching_deals. This can be queried directly:

{resource.service.name="solver" && name="get_targeted_deal"}

Use this query to observe targeting attempts that fail when the requested target is not available.

Details

We use two different methods for tracing a function. When the function does not contain a child span, we wrap the call with start and done span events. For example:

lilypad/pkg/solver/matcher/matcher.go

Lines 167 to 175 in fbc0382

	span.AddEvent("add_match_decision.start")
	_, err := db.AddMatchDecision(matchingResourceOffer.ID, jobOffer.ID, addDealID, true)
	if err != nil {
	span.SetStatus(codes.Error, "unable to add match decision")
	span.RecordError(err)
	return nil, err
	}
	span.AddEvent("add_match_decision.done")
	}

When the function contains a child span, we simply call it passing the context and tracer. For example:

lilypad/pkg/solver/controller.go

Line 282 in fbc0382

deals, err := matcher.GetMatchingDeals(ctx, controller.store, controller.updateJobOfferState, controller.tracer)

The two approaches give us visibility on each function call, either as a pair of events in a span or a child span.

Related issues or PRs (optional)

Implements #75

[bgins]

It may be confusing that that GetResourceOfferIDs has a different behavior than GetResourceOfferID. Open to naming suggestions.

[bgins]

We don't currently implement per module pricing, but this sets us up for it.

[bgins]

In general, span attributes should be set when the span is created because they are generally considered static information. But we're setting them dynamically here as a part of the initialization leading up to the real work.

Setting these values as span attributes makes it easier to query spans with them.

[bgins]

Future work. Would like to refactor this part of the algorithm out into a function. The change would make reading this code easier and we could avoid defining an child span in the scope of its parent.

[walkah]

LGTM and local testing worked. One question, but otherwise great work! 🎉

[walkah]

why did you move this?

[bgins]

Moved it to access it in the price mismatch case:

lilypad/pkg/solver/matcher/match.go

Lines 269 to 273 in 60f45b4

	return &priceMismatch{
	jobOffer: jobOffer,
	resourceOffer: resourceOffer,
	moduleID: moduleID,
	}

This price mismatch case is one scope out from where we were getting the module ID. Moving it avoids recomputing the module ID for this case.