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

Refactor update opentelemetry-go & faiss #2303

Merged

vankichi merged 1 commit into main from refactor/internal-o11y/update-otel

Feb 6, 2024

Collaborator

kpango commented Jan 22, 2024 •

edited by vankichi

Loading

Description:

This PR contains two changes.

addition of a Faiss Agent similar to that in PR2047
- please ignore these changes.
adoption of the latest version of OpenTelemetry

Related Issue:

Versions:

Go Version: 1.21.5
Docker Version: 20.10.8
Kubernetes Version: v1.28.4
NGT Version: 2.1.6

Checklist:

I have read the CONTRIBUTING document and completed our CLA agreement.
I have checked open Pull Requests for the similar feature or fixes?

Special notes for your reviewer:

cloudflare-pages bot commented Jan 22, 2024 •

edited

Loading

Deploying with Cloudflare Pages

Latest commit:	`b48034a`
Status:	✅ Deploy successful!
Preview URL:	https://ac7532e6.vald.pages.dev
Branch Preview URL:	https://refactor-internal-o11y-updat.vald.pages.dev

Collaborator

vdaas-ci commented Jan 22, 2024

[WARNING:INTCFG] Changes in interal/config may require you to change Helm charts. Please check.

github-actions bot reviewed

View reviewed changes

dockers/agent/core/faiss/Dockerfile

+              ENV APP_NAME faiss
+              # skipcq: DOK-DL3008
+              RUN apt-get update && apt-get install -y --no-install-recommends \

Contributor

github-actions bot Jan 22, 2024

⚠️ [hadolint] <DL3008> _{reported by reviewdog 🐶}
Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

github-actions bot added team/set team/sre area/discoverer area/manager/index area/internal type/ci area/helm area/makefile area/agent/core area/gateway/lb area/gateway/filter size/XXXL type/refactoring labels

deepsource-autofix bot added a commit that referenced this pull request


          style: format code with Gofumpt and Prettier

31e5fb1

This commit fixes the style issues introduced in 869a5c8 according to the output
from Gofumpt and Prettier.

Details: #2303

Collaborator

vdaas-ci commented Jan 22, 2024

[CHATOPS:HELP] ChatOps commands.

🙆‍♀️ /approve - approve
🍱 /format - format codes and add licenses
✅ /gen-test - generate test codes
🏷️ /label - add labels
🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

github-actions bot reviewed

View reviewed changes

docs/tutorial/get-started-with-faiss-agent.md

+                 gateway:
+                   lb:
+                     ...
+                     ingress:

Contributor

github-actions bot Jan 22, 2024

[LanguageTool] _{reported by reviewdog 🐶}
This sentence does not start with an uppercase letter. (UPPERCASE_SENTENCE_START)
Suggestions: Ingress
URL: https://languagetool.org/insights/post/spelling-capital-letters/
Rule: https://community.languagetool.org/rule/show/UPPERCASE_SENTENCE_START?lang=en-US
Category: CASING

docs/tutorial/get-started-with-faiss-agent.md

+                     ingress:
+                       enabled: true
+                       # TODO: Set your ingress host.
+                       host: localhost

Contributor

github-actions bot Jan 22, 2024

[LanguageTool] _{reported by reviewdog 🐶}
This sentence does not start with an uppercase letter. (UPPERCASE_SENTENCE_START)
Suggestions: Host
URL: https://languagetool.org/insights/post/spelling-capital-letters/
Rule: https://community.languagetool.org/rule/show/UPPERCASE_SENTENCE_START?lang=en-US
Category: CASING

docs/tutorial/get-started-with-faiss-agent.md

+                       # TODO: Set your ingress host.
+                       host: localhost
+                       # TODO: Set annotations which you have to set for your k8s cluster.
+                       annotations:

Contributor

github-actions bot Jan 22, 2024

[LanguageTool] _{reported by reviewdog 🐶}
This sentence does not start with an uppercase letter. (UPPERCASE_SENTENCE_START)
Suggestions: Annotations
URL: https://languagetool.org/insights/post/spelling-capital-letters/
Rule: https://community.languagetool.org/rule/show/UPPERCASE_SENTENCE_START?lang=en-US
Category: CASING

docs/tutorial/get-started-with-faiss-agent.md

+                 Deploy vald on your Kubernetes cluster.
+                 ```bash
+                 helm install vald vald/vald --values example/helm/values.yaml

Contributor

github-actions bot Jan 22, 2024

[LanguageTool] _{reported by reviewdog 🐶}
Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Suggestions: vald
Rule: https://community.languagetool.org/rule/show/ENGLISH_WORD_REPEAT_RULE?lang=en-US
Category: MISC

docs/tutorial/get-started-with-faiss-agent.md

+                  <details><summary>The detailed explanation of example code is here</summary><br>
+                  This will execute 6 steps.
+.  init

Contributor

github-actions bot Jan 22, 2024

[LanguageTool] _{reported by reviewdog 🐶}
This sentence does not start with an uppercase letter. (UPPERCASE_SENTENCE_START)
Suggestions: Init
URL: https://languagetool.org/insights/post/spelling-capital-letters/
Rule: https://community.languagetool.org/rule/show/UPPERCASE_SENTENCE_START?lang=en-US
Category: CASING

docs/tutorial/get-started-with-faiss-agent.md

+                          <details><summary>example code</summary><br>
+                        ```go
+                        for i := range ids [:insertCount] {

Contributor

github-actions bot Jan 22, 2024

[LanguageTool] _{reported by reviewdog 🐶}
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: IDs
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING

docs/tutorial/get-started-with-faiss-agent.md

+                        for i := range ids [:insertCount] {
+                            _, err := client.Insert(ctx, &payload.Insert_Request{
+                                Vector: &payload.Object_Vector{
+                                    Id:     ids[i],

Contributor

github-actions bot Jan 22, 2024

[LanguageTool] _{reported by reviewdog 🐶}
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: IDs
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING

docs/tutorial/get-started-with-faiss-agent.md

+                                glg.Fatal(err)
+                            }
+                            b, _ := json.MarshalIndent(res.GetResults(), "", " ")

Contributor

github-actions bot Jan 22, 2024

[LanguageTool] _{reported by reviewdog 🐶}
This sentence does not start with an uppercase letter. (UPPERCASE_SENTENCE_START)
Suggestions: B
URL: https://languagetool.org/insights/post/spelling-capital-letters/
Rule: https://community.languagetool.org/rule/show/UPPERCASE_SENTENCE_START?lang=en-US
Category: CASING

docs/tutorial/get-started-with-faiss-agent.md

+                          <details><summary>example code</summary><br>
+                        ```go
+                        for i := range ids [:insertCount] {

Contributor

github-actions bot Jan 22, 2024

[LanguageTool] _{reported by reviewdog 🐶}
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: IDs
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING

docs/tutorial/get-started-with-faiss-agent.md

+                        for i := range ids [:insertCount] {
+                            _, err := client.Remove(ctx, &payload.Remove_Request{
+                                Id: &payload.Object_ID{
+                                    Id: ids[i],

Contributor

github-actions bot Jan 22, 2024

[LanguageTool] _{reported by reviewdog 🐶}
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: IDs
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING

github-actions bot added the priority/low label

kpango pushed a commit that referenced this pull request


          from Gofumpt and Prettier.

04d6fd1

Details: #2303
Signed-off-by: kpango <kpango@vdaas.org>

kpango force-pushed the refactor/internal-o11y/update-otel branch 3 times, most recently from b2b4c77 to 91c1c22 Compare

January 22, 2024 09:19

codecov bot commented Jan 22, 2024 •

edited

Loading

Codecov Report

Attention: 428 lines in your changes are missing coverage. Please review.

Comparison is base (adf1b61) 30.29% compared to head (b48034a) 30.29%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
...ternal/observability/metrics/agent/core/ngt/ngt.go	0.00%	105 Missing ⚠️
...nal/observability/metrics/agent/sidecar/sidecar.go	0.00%	45 Missing ⚠️
...ability/metrics/index/job/correction/correction.go	0.00%	39 Missing ⚠️
...ernal/observability/metrics/manager/index/index.go	0.00%	39 Missing ⚠️
internal/observability/metrics/grpc/grpc.go	0.00%	22 Missing ⚠️
...rvability/metrics/circuitbreaker/circuitbreaker.go	0.00%	20 Missing ⚠️
internal/observability/metrics/backoff/backoff.go	0.00%	19 Missing ⚠️
...servability/metrics/runtime/goroutine/goroutine.go	0.00%	19 Missing ⚠️
internal/config/faiss.go	0.00%	18 Missing ⚠️
internal/observability/metrics/runtime/cgo/cgo.go	0.00%	18 Missing ⚠️
... and 17 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2303      +/-   ##
==========================================
- Coverage   30.29%   30.29%   -0.01%     
==========================================
  Files         391      393       +2     
  Lines       40953    40954       +1     
==========================================
- Hits        12408    12405       -3     
- Misses      27882    27883       +1     
- Partials      663      666       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kpango force-pushed the refactor/internal-o11y/update-otel branch 5 times, most recently from 0b7621c to 56a5637 Compare

January 23, 2024 02:30

kpango force-pushed the refactor/internal-o11y/update-otel branch 7 times, most recently from 17e863a to 76c1447 Compare

February 6, 2024 04:30

ykadowak reviewed

View reviewed changes

Contributor

ykadowak left a comment

We should just fix this in this PR
https://github.com/vdaas/vald/actions/runs/7794733994/job/21256560802?pr=2303#step:6:819

kpango force-pushed the refactor/internal-o11y/update-otel branch 6 times, most recently from 3717ba4 to 8d1e987 Compare

February 6, 2024 07:13


          refactor update opentelemetry-go & faiss

b48034a

Signed-off-by: kpango <kpango@vdaas.org>

kpango force-pushed the refactor/internal-o11y/update-otel branch from 8d1e987 to b48034a Compare

February 6, 2024 07:35

ykadowak approved these changes

View reviewed changes

hlts2 approved these changes

View reviewed changes

Contributor

hlts2 left a comment

LGTM!

vankichi approved these changes

View reviewed changes

vankichi merged commit 88efc1a into main

158 of 160 checks passed

vankichi deleted the refactor/internal-o11y/update-otel branch

February 6, 2024 08:13

kpango added a commit that referenced this pull request


          refactor update opentelemetry-go & faiss (#2303)

f38d3a8

Signed-off-by: kpango <kpango@vdaas.org>

kpango added a commit that referenced this pull request


          refactor update opentelemetry-go & faiss (#2303)

654c33e

format yaml

Signed-off-by: kpango <kpango@vdaas.org>

kpango added a commit that referenced this pull request


          refactor update opentelemetry-go & faiss (#2303)

e1abbf7

Signed-off-by: kpango <kpango@vdaas.org>

kpango added a commit that referenced this pull request


          refactor update opentelemetry-go & faiss (#2303)

7c0b11f

format yaml & update go version and dependencies

Signed-off-by: kpango <kpango@vdaas.org>

kpango added a commit that referenced this pull request


          refactor update opentelemetry-go & faiss (#2303)

141515a

Signed-off-by: kpango <kpango@vdaas.org>

kpango added a commit that referenced this pull request


          refactor update opentelemetry-go & faiss (#2303)

33c10d9

format yaml & update go version and dependencies

Signed-off-by: kpango <kpango@vdaas.org>

ykadowak mentioned this pull request

Fix e2e regressions #2327

Merged

2 tasks

This was referenced Feb 15, 2024

[patch] Release v1.7.11 #2355

Merged

[patch] Release v1.7.11 #2358

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

github-actions[bot] github-actions left review comments

vankichi vankichi approved these changes

hlts2 hlts2 approved these changes

ykadowak ykadowak approved these changes

datelier Awaiting requested review from datelier

kmrmt Awaiting requested review from kmrmt