feat: OTel#200
Open
kuraisle wants to merge 10 commits into
Open
Conversation
This reverts commit bfd9ef1.
401 ["indicates that a request was not successful because it lacks valid authentication credentials for the requested resource"](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/401) should have been this anyway I think
anwarfg
reviewed
Jun 17, 2026
| "/search/text-search/coughing", | ||
| ) | ||
|
|
||
| assert response.status_code in [401, 403] |
Collaborator
There was a problem hiding this comment.
Just checking you intended to leave this as either 403 OR 401. The other test now asserts a specific code, so thought that this would follow the same pattern.
anwarfg
requested changes
Jun 17, 2026
| from options.pipeline_options import LLMModel | ||
| from options.base_options import InferenceType | ||
| get_local_weights = pytest.importorskip("components.models.get_local_weights") | ||
| download_model_from_huggingface = pytest.importorskip("components.models.get_local_weights") |
Collaborator
There was a problem hiding this comment.
I think importorskip is for whole modules, not functions.
Also, the second importorskip line here is the same as the first - I suspect a copypasta error
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description
We need to keep an eye on our vegetables, so here's some OTel monitoring.
The OTel instrumentation has defaults for FastAPI, so the app is now attached to an Instrumentor.
The azure app service default is to use an environment variable
APPLICATIONINSIGHTS_CONNECTION_STRINGto attach the collector, so if this is in the environment the azure monitor is configured.I've put it as an extra dependency (as people should have the option not to load the dependency), and updated the container to load the extra.
UPDATE: I had to change some tests and dependencies. I didn't need the specific
opentelemetry-instrumentation-fastapidependency locally (mysterious) and the malformed auth test has started returning 401, which seems right to me anyway. The CI/CD tests still test the local weights features, but I've flagged them to skip if llama-cpp-python isn't installed.Related Issues or other material
Related #184
Closes #184