feat: 修改metadata下的bkdata_link的租户id逻辑 --story=126913310 #7878
Conversation
1StephenCurry1
added
feat
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the tenant ID handling logic in metadata-related resources by replacing dynamic tenant ID retrieval with explicit tenant ID parameters. The main purpose is to standardize how tenant IDs are managed across various API resources.
- Replaced dynamic tenant ID resolution with explicit
bk_tenant_idparameters in serializers - Removed conditional tenant ID logic based on multi-tenant mode settings
- Added a new utility function to resolve tenant IDs using business ID and app name combination
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| bkmonitor/metadata/resources/vm.py | Added bk_tenant_id field to SwitchKafkaCluster serializer and simplified tenant ID handling |
| bkmonitor/metadata/resources/resources.py | Replaced conditional tenant ID logic with explicit bk_tenant_id parameters across multiple resource classes |
| bkmonitor/bkmonitor/utils/tenant.py | Added new function bk_biz_id_and_app_name_to_bk_tenant_id for tenant ID resolution |
| bkmonitor/apm/models/datasource.py | Updated to use the new tenant ID resolution function and pass tenant IDs explicitly |
| bkmonitor/apm/core/handlers/bk_data/virtual_metric.py | Modified to use new tenant ID resolution approach |
| bkmonitor/apm/core/handlers/bk_data/flow.py | Changed from static to instance methods for tenant ID handling |
| bkmonitor/apm/core/discover/precalculation/storage.py | Updated to pass tenant IDs explicitly in metadata calls |
| bkmonitor/apm/core/discover/precalculation/consul_handler.py | Modified to pass tenant ID from application object |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
代码Review总结这个PR主要是将metadata相关接口的租户ID获取逻辑从动态获取改为显式传参，整体改动合理，有以下几个优点： ✅ 优点
🔍 需要关注的点
🚀 建议
整体来说这是一个很好的重构，提高了代码的可维护性和可读性。 |
bkmonitor/bkmonitor/utils/tenant.py
Outdated
|
|
||
| from django.conf import settings | ||
|
|
||
| from apm.models import ApmApplication |
There was a problem hiding this comment.
这里不要这么写，会导致所有模块产生对apm模块的依赖。如果觉得用的多的话，可以扔apm模块下合适的地方
| metric_data_id = self.metric_datasource.bk_data_id | ||
| return resource.metadata.query_data_source(bk_data_id=metric_data_id) | ||
| # 获取租户id | ||
| bk_tenant_id = bk_biz_id_and_app_name_to_bk_tenant_id(app_name=self.app_name, bk_biz_id=self.bk_biz_id) |
There was a problem hiding this comment.
VirtualMetricFlow的application属性不就是ApmApplication么，是不是可以直接获取里面的租户ID？
|
|
||
| @classmethod | ||
| def get_deploy_params(cls, bk_biz_id, data_id, operator, name, deploy_description=None, extra_maintainers=None): | ||
| def get_deploy_params(self, bk_biz_id, data_id, operator, name, deploy_description=None, extra_maintainers=None): |
There was a problem hiding this comment.
这里把基类改了，后续继承的类override之后对不上了吧？
|
|
||
| def _create_deploy(self): | ||
| params = ApmFlow.get_deploy_params( | ||
| apm_flow_ins = ApmFlow( |
There was a problem hiding this comment.
还是别把classmethod改了吧，容易改坏
bkmonitor/bkmonitor/utils/tenant.py
Outdated
| return space.bk_tenant_id | ||
|
|
||
|
|
||
| def bk_biz_id_and_app_name_to_bk_tenant_id(bk_biz_id: int, app_name: str) -> str: |
There was a problem hiding this comment.
apm的application本来就不会跨业务，这里有业务ID了为什么还必须查一次ApmApplication
| return res | ||
|
|
||
| @classmethod | ||
| def update_result_table(cls, table_name, storage_cluster_id): |
There was a problem hiding this comment.
我看只有一个地方调用了这个方法，而且那个地方已经有了ESStorage，这里为什么不直接加参数？
2 participants
No description provided.