-
Notifications
You must be signed in to change notification settings - Fork 360
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
feat: add subscribed message metrics instrumentations #2080
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2080 +/- ##
==========================================
- Coverage 64.42% 64.40% -0.02%
==========================================
Files 109 109
Lines 15049 15094 +45
==========================================
+ Hits 9695 9721 +26
- Misses 4752 4770 +18
- Partials 602 603 +1
|
I think you can add a simple e2e test for prometheus package main
import (
"fmt"
"io"
"log"
"net/http"
"strings"
"time"
"github.com/prometheus/common/expfmt"
)
func main() {
_, body, err := HTTPGet(fmt.Sprintf("http://127.0.0.1:%d/stats/prometheus", 9901))
if err != nil {
fmt.Println("get metric error: %v", err)
return
}
reader := strings.NewReader(body)
_, err = (&expfmt.TextParser{}).TextToMetricFamilies(reader)
if err != nil {
fmt.Println("parse metric error: %v body: %s", err, body)
}
}
func HTTPGet(url string) (code int, respBody string, err error) {
log.Println("HTTP GET", url)
client := &http.Client{Timeout: time.Minute}
resp, err := client.Get(url)
if err != nil {
log.Println(err)
return 0, "", err
}
defer resp.Body.Close()
body, err := io.ReadAll(resp.Body)
if err != nil {
log.Println(err)
return 0, "", err
}
respBody = string(body)
code = resp.StatusCode
return code, respBody, nil
} |
05a49b2
to
6b2d605
Compare
@zirain Can you help with it in a sparate PR? |
) { | ||
errChans := make(chan error, 10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason for use 10 for buffer size?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need some buffers to make sure the errChan will not be stuck when observing errors from snapshot.Updates. As for the size, I am not sure about it, a suggested value from chatgpt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add some comments for it, or a TODO: find a suitable value
for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@Xunzhuo are we following metric naming conventions similar to EnvoyProxy ? |
hey @Xunzhuo, ive made some suggestions for now mainly around naming, prefer- would also like to replace |
Signed-off-by: bitliu <bitliu@tencent.com>
6b2d605
to
3876fd2
Compare
3876fd2
to
d6bf031
Compare
can udpate an image of the latest output? |
@zirain updated the pic in PR desc. |
After: #1982
Part of: #700
refer: #2089