add telemetry and fix broken api calls #163
Conversation
There was a problem hiding this comment.
Hi team,
I'm currently reviewing this pull request and will provide feedback in a few minutes. In the meantime, I'm giving you a summary of the pull request's content so that you and other reviewers can quickly get up to speed on the changes and their intent.
This pull request, authored by yudyhendry, aims to add telemetry and fix broken API calls in the applied-ai-engineering-samples repository. The changes include:
- Improved Error Handling: The code now includes more robust error handling in various API calls, preventing unexpected crashes and improving the user experience.
- Added Telemetry: Telemetry has been added to several key functions, allowing for better monitoring and performance analysis of the application. This is implemented using the
google.cloud.aiplatform.telemetrylibrary and theUSER_AGENTconstant defined inutils/consts.py(lines 1-15). - Updated
main.py: Several changes were made togenai-on-vertex-ai/genai-website-mod/backend/main.py:- Added
Optionalimport to line 16. - Added
urllib.parseimport to line 38. - Removed extra whitespace on lines 88-91.
- Added
extract_form_datafunction (lines 132-141) to handle form data extraction. - Removed
hx-triggerattribute from line 183 in the HTML response. - Modified the
/web/magiendpoint (lines 288-307) to use theextract_form_datafunction. - Added a new endpoint
/web/magi-follow(lines 311-328) for follow-up searches. - Modified the
/web/magi-videoendpoint (lines 331-340) to use theextract_form_datafunction.
- Added
- Updated other files: Changes were also made to
genai-on-vertex-ai/genai-website-mod/backend/routers/vertex_llm_video.py,genai-on-vertex-ai/genai-website-mod/backend/routers/vertex_search.py,genai-on-vertex-ai/genai-website-mod/backend/static/html/header.html,genai-on-vertex-ai/genai-website-mod/backend/templates/home.html,genai-on-vertex-ai/genai-website-mod/backend/templates/magi-video.html,genai-on-vertex-ai/genai-website-mod/backend/templates/magi.html,genai-on-vertex-ai/genai-website-mod/backend/utils/consts.py,genai-on-vertex-ai/genai-website-mod/backend/utils/content_utils.py,genai-on-vertex-ai/genai-website-mod/backend/utils/imagen.py, andgenai-on-vertex-ai/genai-website-mod/backend/utils/vertex_llm_utils.pyandgenai-on-vertex-ai/genai-website-mod/backend/utils/vertex_search_utils.pyto add telemetry and improve functionality. Specific changes within these files are detailed in the diff. - UI Improvements: The UI has been slightly simplified by removing redundant elements, resulting in a cleaner and more user-friendly interface.
I'll provide a more detailed review shortly.
Meanwhile, here's a little poem for your enjoyment:
Code flows like a river,
Telemetry, a guiding star,
Bugs flee in the night.
Thanks!
There was a problem hiding this comment.
This pull request adds telemetry and fixes some broken API calls. The changes are generally well-structured and address the described issues. The addition of telemetry using a context manager is a good practice. A few minor improvements can enhance the code's readability and maintainability.
| def extract_form_data(form:str) -> dict: | ||
| kvs = {} | ||
| for kv in form.split("&"): | ||
| key = kv.split("=")[0] | ||
| k = parse.unquote(key, encoding="utf-8", errors="replace") | ||
| value = kv.split("=")[1] | ||
| v = parse.unquote(value, encoding="utf-8", errors="replace") | ||
| if value != "": | ||
| kvs[k] = v | ||
| return kvs |
There was a problem hiding this comment.
The extract_form_data function can be simplified using urllib.parse.parse_qs. This built-in function directly handles the parsing of URL-encoded forms, making the code more concise and efficient.
| def extract_form_data(form:str) -> dict: | |
| kvs = {} | |
| for kv in form.split("&"): | |
| key = kv.split("=")[0] | |
| k = parse.unquote(key, encoding="utf-8", errors="replace") | |
| value = kv.split("=")[1] | |
| v = parse.unquote(value, encoding="utf-8", errors="replace") | |
| if value != "": | |
| kvs[k] = v | |
| return kvs | |
| def extract_form_data(form: str) -> dict: | |
| return parse.parse_qs(form) |
| form=query.decode(encoding="utf-8") | ||
| ) | ||
|
|
||
| print("Entering /web/magi: %s", form_data) |
There was a problem hiding this comment.
While print statements are helpful for debugging, it's generally better to use a logging framework for production code. This allows for more structured logging and easier management of log levels.
logging.debug(f"Entering /web/magi: {form_data}")| <input style="border: 1px solid rgb(228, 204, 204); height: 40px; width: 510px;" type="text" | ||
| id="search-bar-query-video" name="video_query" title="Search" type="search" placeholder="Ask Video ..." | ||
| hx-post="/web/magi-video" hx-indicator="#loading-initial-video" hx-include="#search-bar-query-video" | ||
| hx-target="#magi-container-target2" aria-label="Search" /> |
There was a problem hiding this comment.
To maintain consistency with the search functionality in the header, consider adding hx-trigger="submit" to the video search input. This ensures that the search is triggered upon form submission, providing a unified user experience.
| <input style="border: 1px solid rgb(228, 204, 204); height: 40px; width: 510px;" type="text" | |
| id="search-bar-query-video" name="video_query" title="Search" type="search" placeholder="Ask Video ..." | |
| hx-post="/web/magi-video" hx-indicator="#loading-initial-video" hx-include="#search-bar-query-video" | |
| hx-target="#magi-container-target2" aria-label="Search" /> | |
| <input style="border: 1px solid rgb(228, 204, 204); height: 40px; width: 510px;" type="text" | |
| id="search-bar-query-video" name="video_query" title="Search" type="search" placeholder="Ask Video ..." | |
| hx-post="/web/magi-video" hx-indicator="#loading-initial-video" hx-include="#search-bar-query-video" | |
| hx-target="#magi-container-target2" aria-label="Search" hx-trigger="submit" /> |
| <input type="text" id="prompt-follow-up" name="video_query" title="Search" type="search" value="" | ||
| placeholder="Ask ..." aria-label="Search" hx-post="/web/magi-video" | ||
| hx-indicator="#loading-initial-video" hx-include="#prompt-follow-up" | ||
| hx-target="#magi-container-target2" style="width: 100%" /> |
There was a problem hiding this comment.
For consistency with other search inputs and to improve user experience, add hx-trigger="submit" to this input field. This will trigger the search when the user submits the form, aligning the behavior with the main search bar.
| <input type="text" id="prompt-follow-up" name="video_query" title="Search" type="search" value="" | |
| placeholder="Ask ..." aria-label="Search" hx-post="/web/magi-video" | |
| hx-indicator="#loading-initial-video" hx-include="#prompt-follow-up" | |
| hx-target="#magi-container-target2" style="width: 100%" /> | |
| <input type="text" id="prompt-follow-up" name="video_query" title="Search" type="search" value="" | |
| placeholder="Ask ..." aria-label="Search" hx-post="/web/magi-video" | |
| hx-indicator="#loading-initial-video" hx-include="#prompt-follow-up" | |
| hx-target="#magi-container-target2" style="width: 100%" hx-trigger="submit" /> |
| from google.cloud.aiplatform import telemetry | ||
| from utils.consts import USER_AGENT |
There was a problem hiding this comment.
There are duplicate imports of telemetry and USER_AGENT. Remove lines 37 and 38 to clean up the imports.
| from google.cloud.aiplatform import telemetry | ||
| from google.cloud.aiplatform import telemetry |
There was a problem hiding this comment.
Duplicate import for telemetry, remove one.
Pull-Request Template
Thank you for your contribution! Please provide a brief description of your changes and ensure you've completed the checklist below.
Description
What does this PR do? Why is it necessary?
Fixes # (if applicable)
Checklist
nox -s formatto format the code.aaie_notebook_template.ipynbif submitting a new jupyter notbook.