feat: Integrate Traefik API proxy with Aperture #1977
Conversation
|
🤖 OpenAI Here is the summary of changes: The diff adds a new middleware plugin to integrate Traefik API proxy with Aperture, including a package
Chat with 🤖 OpenAI Bot (
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ctx := context.Background() | ||
|
|
||
| apertureAgentGRPCClient, err := grpcClient(ctx, net.JoinHostPort(agentHost, agentPort)) | ||
| if err != nil { | ||
| log.Fatalf("failed to create flow control client: %v", err) | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
There is a potential issue with the use of log.Fatalf here. If there's an error creating the gRPC client, it will cause the entire process to exit. Instead, you should return an error and let the caller handle it.
| ctx := context.Background() | |
| apertureAgentGRPCClient, err := grpcClient(ctx, net.JoinHostPort(agentHost, agentPort)) | |
| if err != nil { | |
| log.Fatalf("failed to create flow control client: %v", err) | |
| } | |
| func (a *TraefikPlugin) ServeHTTP(rw http.ResponseWriter, r *http.Request) { | |
| // ... | |
| apertureAgentGRPCClient, err := grpcClient(ctx, net.JoinHostPort(agentHost, agentPort)) | |
| if err != nil { | |
| rw.WriteHeader(http.StatusInternalServerError) | |
| rw.Write([]byte("failed to create flow control client")) | |
| return | |
| } | |
| // ... | |
| } |
|
|
||
| //initialize Aperture Client with the provided options. | ||
| apertureClient, err := aperture.NewClient(ctx, opts) | ||
| if err != nil { | ||
| log.Fatalf("failed to create client: %v", err) | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
Similar to the previous issue, using log.Fatalf here will cause the entire process to exit if there's an error creating the Aperture client. You should return an error and let the caller handle it.
| //initialize Aperture Client with the provided options. | |
| apertureClient, err := aperture.NewClient(ctx, opts) | |
| if err != nil { | |
| log.Fatalf("failed to create client: %v", err) | |
| } | |
| func (a *TraefikPlugin) ServeHTTP(rw http.ResponseWriter, r *http.Request) { | |
| // ... | |
| apertureClient, err := aperture.NewClient(ctx, opts) | |
| if err != nil { | |
| rw.WriteHeader(http.StatusInternalServerError) | |
| rw.Write([]byte("failed to create client")) | |
| return | |
| } | |
| // ... | |
| } |
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
LanguageTool
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/RELEASING.md|101 col 1| A comma may be missing after the conjunctive/linking adverb ‘Finally’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA[1])
Suggestions: Finally,
URL: https://languagetool.org/insights/post/linking-words/
Rule: https://community.languagetool.org/rule/show/SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA?lang=en-US&subId=1
Category: PUNCTUATION
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/RELEASING.md|107 col 6| Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Suggestions: releasing,
Rule: https://community.languagetool.org/rule/show/AI_HYDRA_LEO_MISSING_COMMA?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/RELEASING.md|107 col 43| This phrase is redundant. Consider using “outside”. (OUTSIDE_OF[1])
Suggestions: outside
URL: https://languagetool.org/insights/post/wordiness/
Rule: https://community.languagetool.org/rule/show/OUTSIDE_OF?lang=en-US&subId=1
Category: REDUNDANCY
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/RELEASING.md|113 col 44| A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Suggestions: directory,
Rule: https://community.languagetool.org/rule/show/AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/RELEASING.md|120 col 5| A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Suggestions: verified,
Rule: https://community.languagetool.org/rule/show/AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|4 col 11| Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE_2[1])
Suggestions: , so
URL: https://languagetool.org/insights/post/comma-before-and/
Rule: https://community.languagetool.org/rule/show/COMMA_COMPOUND_SENTENCE_2?lang=en-US&subId=1
Category: PUNCTUATION
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|17 col 45| This phrase is redundant (‘I’ stands for ‘interfaces’). Use simply “APIs”. (ACRONYM_TAUTOLOGY[8])
Suggestions: APIs
Rule: https://community.languagetool.org/rule/show/ACRONYM_TAUTOLOGY?lang=en-US&subId=8
Category: REDUNDANCY
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|24 col 26| Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’. (EN_A_VS_AN)
Suggestions: an
URL: https://languagetool.org/insights/post/indefinite-articles/
Rule: https://community.languagetool.org/rule/show/EN_A_VS_AN?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|25 col 27| Two consecutive commas (DOUBLE_PUNCTUATION)
Suggestions: ,
URL: https://languagetool.org/insights/post/punctuation-guide/#what-are-periods
Rule: https://community.languagetool.org/rule/show/DOUBLE_PUNCTUATION?lang=en-US
Category: PUNCTUATION
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|28 col 43| Did you just mean “,” or “.”? (COMMA_PERIOD[1])
Suggestions: ,, .
Rule: https://community.languagetool.org/rule/show/COMMA_PERIOD?lang=en-US&subId=1
Category: PUNCTUATION
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|29 col 75| Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’. (EN_A_VS_AN)
Suggestions: an
URL: https://languagetool.org/insights/post/indefinite-articles/
Rule: https://community.languagetool.org/rule/show/EN_A_VS_AN?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|31 col 24| Articles like ‘the’ are rarely followed by punctuation. A word may be missing after ‘the’, or the punctuation mark may not be necessary. (THE_PUNCT[1])
Suggestions: the
Rule: https://community.languagetool.org/rule/show/THE_PUNCT?lang=en-US&subId=1
Category: GRAMMAR
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|49 col 54| If this is a compound adjective that modifies the following noun, use a hyphen. (EN_COMPOUND_ADJECTIVE_INTERNAL)
Suggestions: backwards-compatible
Rule: https://community.languagetool.org/rule/show/EN_COMPOUND_ADJECTIVE_INTERNAL?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|100 col 54| If this is a compound adjective that modifies the following noun, use a hyphen. (EN_COMPOUND_ADJECTIVE_INTERNAL)
Suggestions: backwards-compatible
Rule: https://community.languagetool.org/rule/show/EN_COMPOUND_ADJECTIVE_INTERNAL?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|57 col 44| A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Suggestions: stable,
Rule: https://community.languagetool.org/rule/show/AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|71 col 54| Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’. (EN_A_VS_AN)
Suggestions: an
URL: https://languagetool.org/insights/post/indefinite-articles/
Rule: https://community.languagetool.org/rule/show/EN_A_VS_AN?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|72 col 49| Two consecutive commas (DOUBLE_PUNCTUATION)
Suggestions: ,
URL: https://languagetool.org/insights/post/punctuation-guide/#what-are-periods
Rule: https://community.languagetool.org/rule/show/DOUBLE_PUNCTUATION?lang=en-US
Category: PUNCTUATION
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|77 col 47| Did you just mean “,” or “.”? (COMMA_PERIOD[1])
Suggestions: ,, .
Rule: https://community.languagetool.org/rule/show/COMMA_PERIOD?lang=en-US&subId=1
Category: PUNCTUATION
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|79 col 26| Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’. (EN_A_VS_AN)
Suggestions: an
URL: https://languagetool.org/insights/post/indefinite-articles/
Rule: https://community.languagetool.org/rule/show/EN_A_VS_AN?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|80 col 46| Articles like ‘the’ are rarely followed by punctuation. A word may be missing after ‘the’, or the punctuation mark may not be necessary. (THE_PUNCT[1])
Suggestions: the
Rule: https://community.languagetool.org/rule/show/THE_PUNCT?lang=en-US&subId=1
Category: GRAMMAR
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|107 col 69| A comma may be missing after the conjunctive/linking adverb ‘Instead’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA[1])
Suggestions: Instead,
URL: https://languagetool.org/insights/post/linking-words/
Rule: https://community.languagetool.org/rule/show/SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA?lang=en-US&subId=1
Category: PUNCTUATION
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|109 col 8| It looks like there might be a possessive apostrophe missing. Consider inserting it. (AI_EN_LECTOR_MISSING_NOUN_POSSESSIVE)
Suggestions: modules'
URL: https://languagetool.org/insights/post/possessive-apostrophe/
Rule: https://community.languagetool.org/rule/show/AI_EN_LECTOR_MISSING_NOUN_POSSESSIVE?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|110 col 55| A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Suggestions: stable,
Rule: https://community.languagetool.org/rule/show/AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|118 col 16| A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Suggestions: modules,
Rule: https://community.languagetool.org/rule/show/AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|120 col 66| It seems likely that a singular genitive (’s) apostrophe is missing. (AI_HYDRA_LEO_APOSTROPHE_S_XS)
Suggestions: project's
Rule: https://community.languagetool.org/rule/show/AI_HYDRA_LEO_APOSTROPHE_S_XS?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|121 col 6| Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Suggestions: release,
Rule: https://community.languagetool.org/rule/show/AI_HYDRA_LEO_MISSING_COMMA?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|126 col 13| Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Suggestions: release,
Rule: https://community.languagetool.org/rule/show/AI_HYDRA_LEO_MISSING_COMMA?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|132 col 53| A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Suggestions: policy,
Rule: https://community.languagetool.org/rule/show/AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|143 col 51| Articles like ‘the’ are rarely followed by punctuation. A word may be missing after ‘the’, or the punctuation mark may not be necessary. (THE_PUNCT[1])
Suggestions: the
Rule: https://community.languagetool.org/rule/show/THE_PUNCT?lang=en-US&subId=1
Category: GRAMMAR
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|143 col 68| Two consecutive commas (DOUBLE_PUNCTUATION)
Suggestions: ,
URL: https://languagetool.org/insights/post/punctuation-guide/#what-are-periods
Rule: https://community.languagetool.org/rule/show/DOUBLE_PUNCTUATION?lang=en-US
Category: PUNCTUATION
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|145 col 43| It appears that a noun is missing after “The”. (THE_CC[1])
Rule: https://community.languagetool.org/rule/show/THE_CC?lang=en-US&subId=1
Category: GRAMMAR
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|146 col 41| Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Suggestions: development,
Rule: https://community.languagetool.org/rule/show/AI_HYDRA_LEO_MISSING_COMMA?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|149 col 62| Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE[1])
Suggestions: , so
URL: https://languagetool.org/insights/post/grammar-what-are-compound-sentences/
Rule: https://community.languagetool.org/rule/show/COMMA_COMPOUND_SENTENCE?lang=en-US&subId=1
Category: PUNCTUATION
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|150 col 48| A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Suggestions: done,
Rule: https://community.languagetool.org/rule/show/AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|157 col 1| It appears that a noun is missing after “The”. (THE_CC[1])
Rule: https://community.languagetool.org/rule/show/THE_CC?lang=en-US&subId=1
Category: GRAMMAR
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|169 col 39| You have used the passive voice repeatedly in nearby sentences. To make your writing clearer and easier to read, consider using active voice. (REP_PASSIVE_VOICE[6])
URL: https://languagetool.org/insights/post/what-is-passive-voice/
Rule: https://community.languagetool.org/rule/show/REP_PASSIVE_VOICE?lang=en-US&subId=6
Category: REPETITIONS_STYLE
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|172 col 36| You have used the passive voice repeatedly in nearby sentences. To make your writing clearer and easier to read, consider using active voice. (REP_PASSIVE_VOICE[7])
URL: https://languagetool.org/insights/post/what-is-passive-voice/
Rule: https://community.languagetool.org/rule/show/REP_PASSIVE_VOICE?lang=en-US&subId=7
Category: REPETITIONS_STYLE
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|173 col 12| Possible missing article found. (AI_HYDRA_LEO_MISSING_A)
Suggestions: a version
Rule: https://community.languagetool.org/rule/show/AI_HYDRA_LEO_MISSING_A?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|187 col 66| Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE_2[4])
Suggestions: , and
Rule: https://community.languagetool.org/rule/show/COMMA_COMPOUND_SENTENCE_2?lang=en-US&subId=4
Category: PUNCTUATION
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|204 col 16| It appears that a noun is missing after “the”. (THE_CC[1])
Rule: https://community.languagetool.org/rule/show/THE_CC?lang=en-US&subId=1
Category: GRAMMAR
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|216 col 21| The grammatical number of this noun doesn’t look right. Consider replacing it. (AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
Suggestions: versions
Rule: https://community.languagetool.org/rule/show/AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER?lang=en-US
Category: MISC
integrations/traefik-plugin/vendor/go.opentelemetry.io/otel/VERSIONING.md|217 col 25| Possible missing article found. (AI_HYDRA_LEO_MISSING_A)
Suggestions: a new
Rule: https://community.languagetool.org/rule/show/AI_HYDRA_LEO_MISSING_A?lang=en-US
Category: MISC
| This is a Go port of the exponential backoff algorithm from [Google's HTTP Client Library for Java][google-http-java-client]. | ||
|
|
||
| [Exponential backoff][exponential backoff wiki] | ||
| is an algorithm that uses feedback to multiplicatively decrease the rate of some process, |
There was a problem hiding this comment.
[LanguageTool] reported by reviewdog 🐶
Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM[1])
Suggestions: to
URL: https://languagetool.org/insights/post/synonyms-of-in-order-to/
Rule: https://community.languagetool.org/rule/show/IN_ORDER_TO_PREMIUM?lang=en-US&subId=1
Category: REDUNDANCY
| # Contributing | ||
|
|
||
| Logr is open to pull-requests, provided they fit within the intended scope of | ||
| the project. Specifically, this library aims to be VERY small and minimalist, |
There was a problem hiding this comment.
[LanguageTool] reported by reviewdog 🐶
As an alternative to the over-used intensifier ‘very’, consider replacing this phrase. (EN_WEAK_ADJECTIVE)
Suggestions: Minimal, Tiny, Petite, Small
URL: https://languagetool.org/insights/post/style-another-word-for-very/
Rule: https://community.languagetool.org/rule/show/EN_WEAK_ADJECTIVE?lang=en-US
Category: STYLE
| # A minimal logging API for Go | ||
|
|
||
| [](https://pkg.go.dev/github.com/go-logr/logr) | ||
|
|
There was a problem hiding this comment.
[LanguageTool] reported by reviewdog 🐶
This sentence does not start with an uppercase letter. (UPPERCASE_SENTENCE_START)
Suggestions: Logr
URL: https://languagetool.org/insights/post/spelling-capital-letters/
Rule: https://community.languagetool.org/rule/show/UPPERCASE_SENTENCE_START?lang=en-US
Category: CASING
|
|
||
| logr offers an(other) opinion on how Go programs and libraries can do logging | ||
| without becoming coupled to a particular logging implementation. This is not | ||
| an implementation of logging - it is an API. In fact it is two APIs with two |
There was a problem hiding this comment.
[LanguageTool] reported by reviewdog 🐶
Consider inserting a comma here: “In fact,” (IN_FACT_COMMA[1])
Suggestions: In fact,
URL: https://dictionary.cambridge.org/grammar/british-grammar/in-fact
Rule: https://community.languagetool.org/rule/show/IN_FACT_COMMA?lang=en-US&subId=1
Category: PUNCTUATION
| Many people assert that libraries should not be logging, and as such efforts | ||
| like this are pointless. Those people are welcome to convince the authors of | ||
| the tens-of-thousands of libraries that *DO* write logs that they are all | ||
| wrong. In the meantime, logr takes a more practical approach. |
There was a problem hiding this comment.
[LanguageTool] reported by reviewdog 🐶
‘In the meantime’ might be wordy. Consider a shorter alternative. (EN_WORDINESS_PREMIUM)
Suggestions: Meanwhile, Simultaneously
URL: https://languagetool.org/insights/post/wordiness/
Rule: https://community.languagetool.org/rule/show/EN_WORDINESS_PREMIUM?lang=en-US
Category: STYLE
| Ensure things look correct before submitting a pull request to include the addition. | ||
|
|
||
| **Note**, the generation code was changed to generate versions >= 1.13. | ||
| To generate versions prior to this, checkout the old release of this repository (i.e. [2fe8861](https://github.com/open-telemetry/opentelemetry-go/commit/2fe8861a24e20088c065b116089862caf9e3cd8b)). |
There was a problem hiding this comment.
[LanguageTool] reported by reviewdog 🐶
‘prior to’ might be wordy. Consider a shorter alternative. (EN_WORDINESS_PREMIUM)
Suggestions: before
URL: https://languagetool.org/insights/post/wordiness/
Rule: https://community.languagetool.org/rule/show/EN_WORDINESS_PREMIUM?lang=en-US
Category: STYLE
| First, decide which module sets will be released and update their versions | ||
| in `versions.yaml`. Commit this change to a new branch. | ||
|
|
||
| Update go.mod for submodules to depend on the new release which will happen in the next step. |
There was a problem hiding this comment.
[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Mod, mod
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING
| First, decide which module sets will be released and update their versions | ||
| in `versions.yaml`. Commit this change to a new branch. | ||
|
|
||
| Update go.mod for submodules to depend on the new release which will happen in the next step. |
There was a problem hiding this comment.
[LanguageTool] reported by reviewdog 🐶
Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Suggestions: release,
Rule: https://community.languagetool.org/rule/show/AI_HYDRA_LEO_MISSING_COMMA?lang=en-US
Category: MISC
|
|
||
| ## Tag | ||
|
|
||
| Once the Pull Request with all the version changes has been approved and merged it is time to tag the merged commit. |
There was a problem hiding this comment.
[LanguageTool] reported by reviewdog 🐶
A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Suggestions: merged,
Rule: https://community.languagetool.org/rule/show/AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA?lang=en-US
Category: MISC
| current `HEAD` of your working directory is not the correct commit. | ||
|
|
||
| 2. Push tags to the upstream remote (not your fork: `github.com/open-telemetry/opentelemetry-go.git`). | ||
| Make sure you push all sub-modules as well. |
There was a problem hiding this comment.
[LanguageTool] reported by reviewdog 🐶
This word is normally spelled as one. (EN_COMPOUNDS)
Suggestions: submodules
URL: https://languagetool.org/insights/post/hyphen/
Rule: https://community.languagetool.org/rule/show/EN_COMPOUNDS?lang=en-US
Category: MISC
|
|
||
| func (a TraefikPlugin) ServeHTTP(rw http.ResponseWriter, r *http.Request) { | ||
|
|
||
| ctx := context.Background() | ||
|
|
||
| apertureAgentGRPCClient, err := grpcClient(ctx, net.JoinHostPort(a.AgentHost, a.AgentPort)) | ||
| if err != nil { | ||
| log.Fatalf("failed to create flow control client: %v", err) | ||
| } | ||
|
|
||
| // Initialize the logger | ||
| logger := stdr.New(log.Default()).WithName("aperture-traefik-plugin") | ||
|
|
||
| opts := aperture.Options{ | ||
| ApertureAgentGRPCClientConn: apertureAgentGRPCClient, | ||
| CheckTimeout: 200 * time.Millisecond, | ||
| Logger: &logger, | ||
| } | ||
|
|
||
| //initialize Aperture Client with the provided options. | ||
| apertureClient, err := aperture.NewClient(ctx, opts) | ||
| if err != nil { | ||
| log.Fatalf("failed to create client: %v", err) | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
In the ServeHTTP method, you are creating a new gRPC client and initializing an Aperture client for every incoming request. This can be inefficient and lead to performance issues. Instead, consider initializing these clients in the New function and storing them as fields in the TraefikPlugin struct.
| func (a TraefikPlugin) ServeHTTP(rw http.ResponseWriter, r *http.Request) { | |
| ctx := context.Background() | |
| apertureAgentGRPCClient, err := grpcClient(ctx, net.JoinHostPort(a.AgentHost, a.AgentPort)) | |
| if err != nil { | |
| log.Fatalf("failed to create flow control client: %v", err) | |
| } | |
| // Initialize the logger | |
| logger := stdr.New(log.Default()).WithName("aperture-traefik-plugin") | |
| opts := aperture.Options{ | |
| ApertureAgentGRPCClientConn: apertureAgentGRPCClient, | |
| CheckTimeout: 200 * time.Millisecond, | |
| Logger: &logger, | |
| } | |
| //initialize Aperture Client with the provided options. | |
| apertureClient, err := aperture.NewClient(ctx, opts) | |
| if err != nil { | |
| log.Fatalf("failed to create client: %v", err) | |
| } | |
| type TraefikPlugin struct { | |
| next http.Handler | |
| ControlPoint string | |
| AgentHost string | |
| AgentPort string | |
| apertureClient *aperture.Client | |
| } | |
| func New(ctx context.Context, next http.Handler, config *Config, name string) (http.Handler, error) { | |
| apertureAgentGRPCClient, err := grpcClient(ctx, net.JoinHostPort(config.AgentHost, config.AgentPort)) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to create flow control client: %v", err) | |
| } | |
| logger := stdr.New(log.Default()).WithName("aperture-traefik-plugin") | |
| opts := aperture.Options{ | |
| ApertureAgentGRPCClientConn: apertureAgentGRPCClient, | |
| CheckTimeout: 200 * time.Millisecond, | |
| Logger: &logger, | |
| } | |
| apertureClient, err := aperture.NewClient(ctx, opts) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to create client: %v", err) | |
| } | |
| return &TraefikPlugin{ | |
| next: next, | |
| ControlPoint: config.ControlPoint, | |
| AgentHost: config.AgentHost, | |
| AgentPort: config.AgentPort, | |
| apertureClient: apertureClient, | |
| }, nil | |
| } |
|
|
||
| labels := aperture.LabelsFromCtx(r.Context()) | ||
|
|
||
| for key, value := range r.Header { | ||
| if strings.HasPrefix(key, ":") { | ||
| continue | ||
| } | ||
| labels[key] = strings.Join(value, ",") | ||
| } | ||
|
|
||
| protocol := flowcontrolhttp.SocketAddress_TCP | ||
|
|
||
| sourceHost, sourcePort := aperturemiddlewares.SplitAddress(apertureClient.GetLogger(), r.RemoteAddr) | ||
|
|
||
| destinationPort := uint32(0) | ||
| destinationHost := aperturemiddlewares.GetLocalIP(apertureClient.GetLogger()) | ||
|
|
||
| bodyBytes, err := aperturemiddlewares.ReadClonedBody(r) | ||
| if err != nil { | ||
| apertureClient.GetLogger().V(2).Info("Error reading body", "error", err) | ||
| } | ||
|
|
||
| req := &flowcontrolhttp.CheckHTTPRequest{ | ||
| Source: &flowcontrolhttp.SocketAddress{ | ||
| Protocol: protocol, | ||
| Address: sourceHost, | ||
| Port: sourcePort, | ||
| }, | ||
| Destination: &flowcontrolhttp.SocketAddress{ | ||
| Protocol: protocol, | ||
| Address: destinationHost, | ||
| Port: destinationPort, | ||
| }, | ||
| ControlPoint: a.ControlPoint, | ||
| Request: &flowcontrolhttp.CheckHTTPRequest_HttpRequest{ | ||
| Method: r.Method, | ||
| Path: r.URL.Path, | ||
| Host: r.Host, | ||
| Headers: labels, | ||
| Scheme: r.URL.Scheme, | ||
| Size: r.ContentLength, | ||
| Protocol: r.Proto, | ||
| Body: string(bodyBytes), | ||
| }, | ||
| } | ||
|
|
||
| flow, err := apertureClient.StartHTTPFlow(r.Context(), req) | ||
| if err != nil { | ||
| apertureClient.GetLogger().Info("Aperture flow control got error. Returned flow defaults to Allowed.", "flow.Accepted()", flow.Accepted()) | ||
| } | ||
|
|
||
| if flow.Accepted() { | ||
| a.next.ServeHTTP(rw, r) | ||
| // Need to call End() on the Flow in order to provide telemetry to Aperture Agent for completing the control loop. | ||
| // The first argument captures whether the feature captured by the Flow was successful or resulted in an error. | ||
| // The second argument is error message for further diagnosis. | ||
| err := flow.End(aperture.OK) | ||
| if err != nil { | ||
| apertureClient.GetLogger().Info("Aperture flow control end got error.", "error", err) | ||
| } | ||
| } else { | ||
| resp := flow.CheckResponse().GetDeniedResponse() | ||
|
|
||
| if resp == nil { | ||
| rw.WriteHeader(http.StatusServiceUnavailable) | ||
| } else { | ||
| rw.WriteHeader(int(resp.GetStatus())) | ||
| for key, value := range resp.GetHeaders() { | ||
| rw.Header().Set(key, value) | ||
| } | ||
|
|
||
| _, err := fmt.Fprint(rw, resp.GetBody()) | ||
| if err != nil { | ||
| apertureClient.GetLogger().Info("Aperture flow control respond body got an error.", "error", err) | ||
| } | ||
| } | ||
|
|
||
| err = flow.End(aperture.OK) | ||
| if err != nil { | ||
| apertureClient.GetLogger().Info("Aperture flow control end got error.", "error", err) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
Now that the Aperture client is initialized in the New function, you can remove the initialization code from the ServeHTTP method and use the apertureClient field directly.
| labels := aperture.LabelsFromCtx(r.Context()) | |
| for key, value := range r.Header { | |
| if strings.HasPrefix(key, ":") { | |
| continue | |
| } | |
| labels[key] = strings.Join(value, ",") | |
| } | |
| protocol := flowcontrolhttp.SocketAddress_TCP | |
| sourceHost, sourcePort := aperturemiddlewares.SplitAddress(apertureClient.GetLogger(), r.RemoteAddr) | |
| destinationPort := uint32(0) | |
| destinationHost := aperturemiddlewares.GetLocalIP(apertureClient.GetLogger()) | |
| bodyBytes, err := aperturemiddlewares.ReadClonedBody(r) | |
| if err != nil { | |
| apertureClient.GetLogger().V(2).Info("Error reading body", "error", err) | |
| } | |
| req := &flowcontrolhttp.CheckHTTPRequest{ | |
| Source: &flowcontrolhttp.SocketAddress{ | |
| Protocol: protocol, | |
| Address: sourceHost, | |
| Port: sourcePort, | |
| }, | |
| Destination: &flowcontrolhttp.SocketAddress{ | |
| Protocol: protocol, | |
| Address: destinationHost, | |
| Port: destinationPort, | |
| }, | |
| ControlPoint: a.ControlPoint, | |
| Request: &flowcontrolhttp.CheckHTTPRequest_HttpRequest{ | |
| Method: r.Method, | |
| Path: r.URL.Path, | |
| Host: r.Host, | |
| Headers: labels, | |
| Scheme: r.URL.Scheme, | |
| Size: r.ContentLength, | |
| Protocol: r.Proto, | |
| Body: string(bodyBytes), | |
| }, | |
| } | |
| flow, err := apertureClient.StartHTTPFlow(r.Context(), req) | |
| if err != nil { | |
| apertureClient.GetLogger().Info("Aperture flow control got error. Returned flow defaults to Allowed.", "flow.Accepted()", flow.Accepted()) | |
| } | |
| if flow.Accepted() { | |
| a.next.ServeHTTP(rw, r) | |
| // Need to call End() on the Flow in order to provide telemetry to Aperture Agent for completing the control loop. | |
| // The first argument captures whether the feature captured by the Flow was successful or resulted in an error. | |
| // The second argument is error message for further diagnosis. | |
| err := flow.End(aperture.OK) | |
| if err != nil { | |
| apertureClient.GetLogger().Info("Aperture flow control end got error.", "error", err) | |
| } | |
| } else { | |
| resp := flow.CheckResponse().GetDeniedResponse() | |
| if resp == nil { | |
| rw.WriteHeader(http.StatusServiceUnavailable) | |
| } else { | |
| rw.WriteHeader(int(resp.GetStatus())) | |
| for key, value := range resp.GetHeaders() { | |
| rw.Header().Set(key, value) | |
| } | |
| _, err := fmt.Fprint(rw, resp.GetBody()) | |
| if err != nil { | |
| apertureClient.GetLogger().Info("Aperture flow control respond body got an error.", "error", err) | |
| } | |
| } | |
| err = flow.End(aperture.OK) | |
| if err != nil { | |
| apertureClient.GetLogger().Info("Aperture flow control end got error.", "error", err) | |
| } | |
| } | |
| } | |
| func (a TraefikPlugin) ServeHTTP(rw http.ResponseWriter, r *http.Request) { | |
| ctx := context.Background() | |
| labels := aperture.LabelsFromCtx(r.Context()) | |
| for key, value := range r.Header { | |
| if strings.HasPrefix(key, ":") { | |
| continue | |
| } | |
| labels[key] = strings.Join(value, ",") | |
| } | |
| protocol := flowcontrolhttp.SocketAddress_TCP | |
| sourceHost, sourcePort := aperturemiddlewares.SplitAddress(a.apertureClient.GetLogger(), r.RemoteAddr) | |
| destinationPort := uint32(0) | |
| destinationHost := aperturemiddlewares.GetLocalIP(a.apertureClient.GetLogger()) | |
| bodyBytes, err := aperturemiddlewares.ReadClonedBody(r) | |
| if err != nil { | |
| a.apertureClient.GetLogger().V(2).Info("Error reading body", "error", err) | |
| } | |
| req := &flowcontrolhttp.CheckHTTPRequest{ | |
| Source: &flowcontrolhttp.SocketAddress{ | |
| Protocol: protocol, | |
| Address: sourceHost, | |
| Port: sourcePort, | |
| }, | |
| Destination: &flowcontrolhttp.SocketAddress{ | |
| Protocol: protocol, | |
| Address: destinationHost, | |
| Port: destinationPort, | |
| }, | |
| ControlPoint: a.ControlPoint, | |
| Request: &flowcontrolhttp.CheckHTTPRequest_HttpRequest{ | |
| Method: r.Method, | |
| Path: r.URL.Path, | |
| Host: r.Host, | |
| Headers: labels, | |
| Scheme: r.URL.Scheme, | |
| Size: r.ContentLength, | |
| Protocol: r.Proto, | |
| Body: string(bodyBytes), | |
| }, | |
| } | |
| flow, err := a.apertureClient.StartHTTPFlow(r.Context(), req) | |
| if err != nil { | |
| a.apertureClient.GetLogger().Info("Aperture flow control got error. Returned flow defaults to Allowed.", "flow.Accepted()", flow.Accepted()) | |
| } | |
| if flow.Accepted() { | |
| a.next.ServeHTTP(rw, r) | |
| err := flow.End(aperture.OK) | |
| if err != nil { | |
| a.apertureClient.GetLogger().Info("Aperture flow control end got error.", "error", err) | |
| } | |
| } else { | |
| resp := flow.CheckResponse().GetDeniedResponse() | |
| if resp == nil { | |
| rw.WriteHeader(http.StatusServiceUnavailable) | |
| } else { | |
| rw.WriteHeader(int(resp.GetStatus())) | |
| for key, value := range resp.GetHeaders() { | |
| rw.Header().Set(key, value) | |
| } | |
| _, err := fmt.Fprint(rw, resp.GetBody()) | |
| if err != nil { | |
| a.apertureClient.GetLogger().Info("Aperture flow control respond body got an error.", "error", err) | |
| } | |
| } | |
| err = flow.End(aperture.OK) | |
| if err != nil { | |
| a.apertureClient.GetLogger().Info("Aperture flow control end got error.", "error", err) | |
| } | |
| } | |
| } |
|
This PR - #1971 may be useful for you. |
@openai ignore
Description of change
closes #1804
Checklist
Summary by OpenAI
New Feature:
http.Handlerinterface forTraefikPluginChore:
.gitignoreforbackoff/v4package.golangci.yamlconfiguration file for CI/CD pipeline