8000 feat(api): Add maxcompute sink to log prediction logs for Model Observability by vinoth-gojek · Pull Request #631 · caraml-dev/merlin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(api): Add maxcompute sink to log prediction logs for Model Observability #631

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 25 commits into from
Apr 8, 2025
Merged

feat(api): Add maxcompute sink to log prediction logs for Model Observability #631

merged 25 commits into from
Apr 8, 2025

Conversation

vinoth-gojek
Copy link
Contributor
@vinoth-gojek vinoth-gojek commented Feb 18, 2025 •
edited
Loading

Description

The changes aims to enable MaxCompute as a sink for Model Observability prediction logs

Modifications

  1. Adds MaxCompute config as sink_types when publisher-observer for a model is deployed
  2. Adds changes to create, alter table and write records for observability prediction logs

Note to Reviewer

  1. Work around - At the moment MaxCompute doesn't support adding partitions on Timestamp columns, so we create partition on day with trunc_time(timestamp, 'day')
  2. PYODPS at the moment don't support the below functionality in the SDK, so we use the execute_sql
    a. Creating tables with auto partition on date
    b. Alter table to add new columns
  3. There is one more blocking error from MaxCompute - df.persist doesn't insert rows with overwrite=False throwing an error "Partition doesn't exist"

Tests

Checklist

  • Added PR label
  • Added unit test, integration, and/or e2e tests
  • Tested locally
  • Updated documentation
  • Update Swagger spec if the PR introduce API changes
  • Regenerated Golang and Python client if the PR introduces API changes

@vinoth-gojek vinoth-gojek force-pushed the add_maxcompute_sink branch 2 times, most recently from 9e612d2 to 7fdcad6 Compare February 21, 2025 10:30
@vinoth-gojek vinoth-gojek changed the title [Draft] Add maxcompute sink feat(api): Add maxcompute sink Feb 21, 2025
@vinoth-gojek vinoth-gojek added the enhancement New feature or request label Feb 21, 2025
@vinoth-gojek vinoth-gojek changed the title feat(api): Add maxcompute sink feat(api): Add maxcompute sink to log prediction logs for Model Observability Feb 25, 2025
naufalandika
naufalandika reviewed Feb 26, 2025
View reviewed changes
@vinoth-gojek vinoth-gojek force-pushed the add_maxcompute_sink branch from cc209fa to a5ea8ae Compare March 6, 2025 04:39
@vinoth-gojek vinoth-gojek force-pushed the add_maxcompute_sink branch from 4bd2701 to 659b864 Compare April 1, 2025 09:16
deadlycoconuts
deadlycoconuts requested changes Apr 2, 2025
View reviewed changes
Copy link
Contributor
@deadlycoconuts deadlycoconuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this PR! Most of the comments are pretty minor refactoring changes. The most important one would probably be the one on the creation/copying from temp tables. Let me know what you think about that one :)

@vinoth-gojek vinoth-gojek merged commit 2733235 into main Apr 8, 2025
96 of 100 checks passed
@vinoth-gojek vinoth-gojek deleted the add_maxcompute_sink branch April 8, 2025 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@naufalandika naufalandika naufalandika left review comments

@deadlycoconuts deadlycoconuts deadlycoconuts approved these changes

@mbruner mbruner Awaiting requested review from mbruner

@shydefoo shydefoo Awaiting requested review from shydefoo

@bthari bthari Awaiting requested review from bthari

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vinoth-gojek @deadlycoconuts @naufalandika
0