Adds function to find candidate occurrences via search

[kritzcreek]

Closes https://linear.app/sourcegraph/issue/GRAPH-640/pre-filter-candidate-files-by-consulting-zoektsearcher

I'm wondering what to do about o11y here. Should I be adding something to the operations, or some more logging?

Test plan

Easiest way to test right now is to generate a syntactic index, upload it, pull up the site-admin api console and hit the occurrences API endpoint with:

query {
  usagesForSymbol(range: {
    repository: "github.com/sourcegraph/sourcegraph"
    path: "docker-images/syntax-highlighter/crates/syntax-analysis/testdata/globals.java"
    revision: "...."
    start: {
      line: 21,
      character: 17
    }, end: {
      line: 21,
      character: 30
    }
  }) {
    nodes {
      usageRange {
        repository,
        revision,
        path,
        range {
          start {
            line,
            character
          }
          end {
            line,
            character
          }
        }
      }
      symbol {
        name,
        provenance
      }
    }
  }
}

[varungandhi-src]

I don't quite understand what this means, but if we're only picking the first element of the slice, maybe the function getSyntacticSymbolsAtRange shouldn't be returning a slice?

[varungandhi-src]

I'm wondering if we should handle a context timeout error in a special way here (or in the outermost loop maybe?) 🤔

[varungandhi-src]

I'm wondering what to do about o11y here. Should I be adding something to the operations, or some more logging?

We should add an operation for each public method (at least, that's the pattern we're following for now). So yes, we should have an operation for SyntacticUsages. For the cross-service call, the search code will handle creating a span, so we're good there. However, it's generally useful to attach extra information about the results to a final event. How many files were matched, total number of matches etc. We should also attach an event related to the query details, it looks like the Execute function doesn't attach any information related to that.

[kritzcreek]

I handled most of the feedback, but decided after a while that some of it is not actually required for the MVP stage. I've filed a follow-up issue for me to come back for some of these https://linear.app/sourcegraph/issue/GRAPH-683/syntactic-usages-tech-debt

[varungandhi-src]

Will do another pass after an hour or so.

[varungandhi-src]

I just looked into this; it looks like endObservation will log the error if we pass in a proper pointer here. So if we really want to return SyntacticUsageError from this function, then we can do the following sequence.

Suggested change

	) ([]struct{}, *SyntacticUsagesError) {
	// The `nil` in the second argument is here, because `With` does not work with custom error types.
	ctx, trace, endObservation := s.operations.syntacticUsages.With(ctx, nil, observation.Args{Attrs: []attribute.KeyValue{
	attribute.Int("repoId", int(repo.ID)),
	attribute.String("commit", string(commit)),
	attribute.String("path", path),
	attribute.String("symbolRange", symbolRange.String()),
	}})
	defer endObservation(1, observation.Args{})
	) (_ []struct{}, err *SyntacticUsagesError) {
	var erasedError error
	// The `nil` in the second argument is here, because `With` does not work with custom error types.
	ctx, trace, endObservation := s.operations.syntacticUsages.With(ctx, &erasedError, observation.Args{Attrs: []attribute.KeyValue{
	attribute.Int("repoId", int(repo.ID)),
	attribute.String("commit", string(commit)),
	attribute.String("path", path),
	attribute.String("symbolRange", symbolRange.String()),
	}})
	defer endObservation(1, observation.Args{})
	defer func(){ if err != nil { erasedError = *err } }()

[varungandhi-src]

• The function argument allows returning an error because content-fetching might return an error. If we don't have access to the content here, you can pass nil for the second argument. So there won't be any error returned from the API.
• If there are multiple languages returned, we should do an AND instead of taking the first one.

[varungandhi-src]

I updated the tech debt issue with more details. I'm OK with merging this as-is for now, but I don't want us to rush stuff like this purely for demo day, we can share a demo in Slack outside of demo day too.

[kritzcreek]

but I don't want us to rush stuff like this purely for demo day

Nothing about demo day and everything about the MVP milestone. This code doesn't need to deal with language ambiguity to check its viability, but that's what I want to be laser-focused on.

[kritzcreek]

Just want to clarify, I'm not saying I don't appreciate the thorough review, feedback, and discussion. I read every comment and definitely intend to get back to and address the rest of it.
I just decided that the bar for things I want to do right now has to impact the MVP aspect of the project.