Skip to content
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.

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

[INLONG-5903][Sort] Make InLong metric constructs factory more cohesive #5906

Merged
merged 9 commits into from
Sep 19, 2022

Conversation

thesumery
Copy link
Contributor

@thesumery thesumery commented Sep 15, 2022

Prepare a Pull Request

(Change the title refer to the following example)

[INLONG-5903][Sort] Make InLong metric constructs factory more cohesive

(The following XYZ should be replaced by the actual GitHub Issue number)

Motivation

To make inlong sort metric code logic more clearly.

Modifications

Refactor SinkMetricData and SourceMetricData constuct function.
Add MetricOption to replace scattered metric configuration
Modify inlong.audit configuration to metrics.audit.proxy.hosts to keep consistent with manager.

Copy link
Contributor

@EMsnap EMsnap left a comment

Choose a reason for hiding this comment

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

pls test with manager and check the result

Copy link
Contributor

@yunqingmoswu yunqingmoswu left a comment

Choose a reason for hiding this comment

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

Unified processing of inLong -> inlong

Copy link
Contributor

@EMsnap EMsnap left a comment

Choose a reason for hiding this comment

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

I guess inlong audit should not rely on inlongMetric since it only requires inlongGroup and inlongStream

@thesumery
Copy link
Contributor Author

pls test with manager and check the result

image

prometheus result is the same as aduit server database;
image

@gong
Copy link
Contributor

gong commented Sep 19, 2022

mysql new source need modify metric option

1. Add metric state init in MetricOption
2. Mysql new Source refactor with MetricOption
3. Add metric data toString to print some debug help infomation
Copy link
Contributor

@yunqingmoswu yunqingmoswu left a comment

Choose a reason for hiding this comment

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

LGTM

@gong
Copy link
Contributor

gong commented Sep 19, 2022

inlong-sort/sort-connectors/base/src/main/java/org/apache/inlong/sort/base/metric/SinkMetricData.java

    public void invoke(long rowCount, long rowSize) {
        if (numRecordsOut != null) {
            numRecordsOut.inc(rowCount);
        }
        if (numBytesOut != null) {
            numBytesOut.inc(rowSize);
        }
        if (auditImp != null) {
            auditImp.add(
                    Constants.AUDIT_SORT_OUTPUT,
                    getGroupId(),
                    getStreamId(),
                    System.currentTimeMillis(),
                    rowCount,
                    rowSize);
        }
    }

lost numRecordsOutForMeter and numBytesOutForMeter computing

@thesumery
Copy link
Contributor Author

fixed

@dockerzhang dockerzhang merged commit 4fa1218 into apache:master Sep 19, 2022
dockerzhang pushed a commit that referenced this pull request Sep 19, 2022
…ve (#5906)

Co-authored-by: thesumery <158971219@qq.com>
liaorui pushed a commit to liaorui/inlong that referenced this pull request Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improve][Sort] Make InLong metric constructs factory more cohesive
8 participants