-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Consolidated OTel ES IndexNameProviders #2458
Consolidated OTel ES IndexNameProviders #2458
Conversation
Signed-off-by: Joe Elliott <number101010@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #2458 +/- ##
==========================================
+ Coverage 95.57% 95.59% +0.01%
==========================================
Files 208 208
Lines 10690 10690
==========================================
+ Hits 10217 10219 +2
+ Misses 401 399 -2
Partials 72 72
Continue to review full report at Codecov.
|
Signed-off-by: Joe Elliott <number101010@gmail.com>
@pavolloffay If you have a chance to look at this I would appreciate it. |
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.
It looks good but the package should be changed. I am fine to keep it in reader or exporter packages or move it to internal under something else than esclient
.
@@ -12,39 +12,54 @@ | |||
// See the License for the specific language governing permissions and | |||
// limitations under the License. | |||
|
|||
package esspanreader | |||
package esclient |
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.
It should go to a different package than esclient
. The esclient
is dedicated only for the ES client used by Jaeger writer/reader.
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.
moved to ./internal/esutil
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Which problem is this PR solving?
There were two
indexNameProvider
s with very similar code. One for reading and one for writing. These have been consolidated into one provider in theinternal/esutil
packageShort description of the changes
IndexNameProvider
s and tests