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

Provide customization to allow passing X-Geo-* headers into gRPC metadata #201

Merged
merged 5 commits into from Oct 20, 2020
Merged

Provide customization to allow passing X-Geo-* headers into gRPC metadata #201

merged 5 commits into from Oct 20, 2020

Conversation

burov
Copy link

@burov burov commented Oct 13, 2020

No description provided.

@burov burov requested a review from Evgeniy-L October 13, 2020 15:22
Copy link
Contributor

@Evgeniy-L Evgeniy-L left a comment

Choose a reason for hiding this comment

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

Looks good

Comment on lines +161 to +163
atlasMatcher := AtlasDefaultHeaderMatcher()
return func(headerName string) (string, bool) {
if key, ok := runtime.DefaultHeaderMatcher(headerName); ok {
if key, ok := atlasMatcher(headerName); ok {
Copy link
Contributor

@kd7lxl kd7lxl Oct 15, 2020

Choose a reason for hiding this comment

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

Rather than plugging the new headers in like this, I think it would be better to implement

func ChainHeaderMatcher(matchers ...runtime.HeaderMatcherFunc)  runtime.HeaderMatcherFunc

so that these custom matchers can be used both independently and together, not just together. It's the same pattern we use to implement interceptors, instead of wrapping interceptors.

Copy link
Author

Choose a reason for hiding this comment

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

It actually a good idea,i will add ChainHeaderMatcher

Copy link
Contributor

Choose a reason for hiding this comment

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

I provided an example of how ChainHeaderMatcher() can simplify this code in #202

@burov burov merged commit bb98be7 into infobloxopen:master Oct 20, 2020
abynenkov-ib added a commit that referenced this pull request Apr 27, 2021
* add resource identifiers

Resource IDs are a typed guid proceeded by metadata helpful for
humans to identify what the guid is referring to.

stopping gRPC does not immediately guarantee http server is stopped.
Serve() will call Stop() at the end. Stop() guarantees both servers
are stopped and listeners are closed.

* Expose HandleRequestID (#181)

* Add an option to enable field mask override (#183)

* Add an option to enable field mask override

* Implement option pattern for PresenceClientInterceptor
* Add WithOverrideFieldMask option to enable field mask override

* Fix minor issues

* provide test to override option

* add test for multiple field masks

* make requestid name consistent with JSON standart (#182)

* update grpc

* use >=1.4.1 for grpc-gateway

* Add common logger with specific fields (#188)

* Add common logger with specific fields

* Add common logger, interceptors with customizable fields

* Fix populating context

* Fix unused arguments

* Fix duplicate append ctx call, rename auxilary func name

* Add custom fields from request headers

* Add err handling if subject key is missing

* Add err handling in case request_id missing

* Begin transaction with options. (#189)

* Begin transaction with options.

  * Extended `Transaction` object with support of begging transaction with
    options (to be able to set isolation level and RO option). Added
    `gorm.BeginTx()` wrapper;
  * Added analog wrapper to do same thing while extracting txn from context;
  * Updated UTs.

* * Reduced code duplication in UTs.
  * Added comments.

* Add stream interceptors (#190)

* Add stream interceptors

* Add UTs, fix subject field handling

* Fix tests

* Improve subject value handling

* Remove redundant ctx returns, fix populating fields into cxt logger

* Add notes to generate mocks

* Add blank line

* Logging interceptor refactor (#192)

* use logrus server interceptors

* lint auth package

* move server stream mock outside of requestid for reuse

* add stream server interceptor for account_id logging

* Add FIXME on ClientInterceptor test

When the test correctly supplies Outgoing metadata, AuthFromMD cannot extract the authorization header because it is designed for Incoming MD only.
The GetAccountID() function will need to be rewritten to work in ClientInterceptors.

* Implement tracing package for toolkit (#193)

* add SpanContextAnnotator
SpanContextAnnotator retrieve current trace.Span from context.Context
or, in case there is no open trace.Span assemble trace.SpanContext from HTTP
headers

* Implement wrapper to extend ochttp.Handler

* Add options to configure what should be added to span

* Implement propogation of request/response headers, and body to span

* Implement wrapper to extend ocgrpc.ServerHandler

* Add options to configure what should be added to span

* Implement propogation of inbound/outbound metadata, and payload to span

* add util functions

* add grpc error into span

* implement simple exporter constructor

* implement obfuscation logic

* move obfuscation factor to constant

* Get lattest span value after calling wrapped handler

* Use library function to assemble span context

* Fix request id key (#197)

* fix request id key

* fix formating

* fix tests

* add test whith deprecated request-id key

* Provide customization to allow passing X-Geo-* headers into gRPC metadata (#201)

* dep ensure

* implement AtalsDefaultHeaderMatcher

* use AtlasDefaultHeaderMatcher by default

* Use AtlasDefaultHeaderMatcher in ExtendedHeaderMatcher

* add ChainHeaderMatcher

* Utilize ChainHeaderMatcher (#202)

* Add unit tests (#203)

* B-30111 - Add UTs

* B-30111 - Add UTs

* Add unit tests

* 'unable to get field from token' spam message (#205)

* 'unable to get field from token' spam message

* fix TestHeadersToAttributes map

* propagating context to gorm transaction (#207)

* Migrate atlas toolkit to go mod (#208)

migrated to go mod

updated travis CI

* Refactor Headers. Util to include Headers into outgoing context

* Refactor Headers. Util to include Headers into outgoing context

* Rename util funcs

* Added support for graceful shutdown (#210)

Added support for disabling automatic server stop

* update Go deps with Dependabot (#209)

* Bump github.com/sirupsen/logrus from 1.4.2 to 1.7.0 (#215)

Bumps [github.com/sirupsen/logrus](https://github.com/sirupsen/logrus) from 1.4.2 to 1.7.0.
- [Release notes](https://github.com/sirupsen/logrus/releases)
- [Changelog](https://github.com/sirupsen/logrus/blob/master/CHANGELOG.md)
- [Commits](sirupsen/logrus@v1.4.2...v1.7.0)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump github.com/golang/protobuf from 1.4.2 to 1.4.3 (#216)

Bumps [github.com/golang/protobuf](https://github.com/golang/protobuf) from 1.4.2 to 1.4.3.
- [Release notes](https://github.com/golang/protobuf/releases)
- [Commits](golang/protobuf@v1.4.2...v1.4.3)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump golang from 1.10.0 to 1.15.8 in /tools/debugging (#212)

Bumps golang from 1.10.0 to 1.15.8.

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump github.com/grpc-ecosystem/go-grpc-middleware from 1.2.0 to 1.2.2 (#217)

Bumps [github.com/grpc-ecosystem/go-grpc-middleware](https://github.com/grpc-ecosystem/go-grpc-middleware) from 1.2.0 to 1.2.2.
- [Release notes](https://github.com/grpc-ecosystem/go-grpc-middleware/releases)
- [Changelog](https://github.com/grpc-ecosystem/go-grpc-middleware/blob/master/CHANGELOG.md)
- [Commits](grpc-ecosystem/go-grpc-middleware@v1.2.0...v1.2.2)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump github.com/DATA-DOG/go-sqlmock from 1.4.1 to 1.5.0 (#218)

Bumps [github.com/DATA-DOG/go-sqlmock](https://github.com/DATA-DOG/go-sqlmock) from 1.4.1 to 1.5.0.
- [Release notes](https://github.com/DATA-DOG/go-sqlmock/releases)
- [Commits](DATA-DOG/go-sqlmock@v1.4.1...v1.5.0)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump github.com/jinzhu/gorm from 1.9.12 to 1.9.16 (#213)

Bumps [github.com/jinzhu/gorm](https://github.com/jinzhu/gorm) from 1.9.12 to 1.9.16.
- [Release notes](https://github.com/jinzhu/gorm/releases)
- [Commits](jinzhu/gorm@v1.9.12...v1.9.16)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump github.com/google/uuid from 1.1.1 to 1.2.0 (#219)

Bumps [github.com/google/uuid](https://github.com/google/uuid) from 1.1.1 to 1.2.0.
- [Release notes](https://github.com/google/uuid/releases)
- [Commits](google/uuid@v1.1.1...v1.2.0)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump golang from 1.15.8 to 1.16.0 in /tools/debugging (#223)

Bumps golang from 1.15.8 to 1.16.0.

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump github.com/sirupsen/logrus from 1.7.0 to 1.8.0 (#225)

Bumps [github.com/sirupsen/logrus](https://github.com/sirupsen/logrus) from 1.7.0 to 1.8.0.
- [Release notes](https://github.com/sirupsen/logrus/releases)
- [Changelog](https://github.com/sirupsen/logrus/blob/master/CHANGELOG.md)
- [Commits](sirupsen/logrus@v1.7.0...v1.8.0)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* valid token may not have subject field

* Bump golang from 1.16.0 to 1.16.3 in /tools/debugging

Bumps golang from 1.16.0 to 1.16.3.

Signed-off-by: dependabot[bot] <support@github.com>

* add OldStatusCreatedOnUpdate to support old grpc -> http status mapping

* update comment.

* fix fmt

* Make interceptor accept custom transaction.

Co-authored-by: Drew Wells <drew.wells00@gmail.com>
Co-authored-by: Arash Outadi <aoutadi@infoblox.com>
Co-authored-by: Aliaksei Burau <aburau@infoblox.com>
Co-authored-by: Daniel Garcia <daniel@danielgarcia.info>
Co-authored-by: Daniel Garcia <dgarcia@infoblox.com>
Co-authored-by: Anton Dudko <52532588+addudko@users.noreply.github.com>
Co-authored-by: Maksim Volkov <mvolkov@infoblox.com>
Co-authored-by: Tom Hayward <thayward@infoblox.com>
Co-authored-by: Alexey Bynenkov <54314036+abynenkov-ib@users.noreply.github.com>
Co-authored-by: Pavel Rybintsev <2716513+prybintsev@users.noreply.github.com>
Co-authored-by: Andrei Lavrov <alavrov@infoblox.com>
Co-authored-by: Anton Dudko <a.d.dudko@hotmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: binjip978 <binjip978@gmail.com>
Co-authored-by: Sergei Semenchuk <ssementchouk@infoblox.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants