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

NOISSUE - Trace consume operations #1778

Merged
merged 15 commits into from
Jul 22, 2023
Merged

Conversation

SammyOina
Copy link
Contributor

@SammyOina SammyOina commented Apr 24, 2023

What does this do?

Add tracing on consume operations

Which issue(s) does this PR fix/relate to?

https://github.com/ultravioletrs/issues/issues/210

List any changes that modify/break current functionality

none

Have you included tests for your changes?

none

Did you document any new/modified functionality?

no

Notes

@SammyOina SammyOina requested a review from a team as a code owner April 24, 2023 11:00
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Apr 28, 2023

Codecov Report

Merging #1778 (c523c65) into master (41be96a) will not change coverage.
The diff coverage is 93.10%.

@@           Coverage Diff           @@
##           master    #1778   +/-   ##
=======================================
  Coverage   64.73%   64.73%           
=======================================
  Files         118      118           
  Lines        9702     9702           
=======================================
  Hits         6281     6281           
  Misses       2753     2753           
  Partials      668      668           
Impacted Files Coverage Δ
consumers/notifiers/service.go 60.00% <50.00%> (ø)
consumers/writers/cassandra/consumer.go 77.55% <100.00%> (ø)
consumers/writers/influxdb/consumer.go 89.87% <100.00%> (ø)
consumers/writers/mongodb/consumer.go 79.31% <100.00%> (ø)
consumers/writers/postgres/consumer.go 65.76% <100.00%> (ø)
consumers/writers/timescale/consumer.go 66.99% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@rodneyosodo rodneyosodo left a comment

Choose a reason for hiding this comment

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

On second thought since consumeBlockingOP or consumeAsyncOP are in the same context with consumeMessageOP and when a client publishes a message Channel Subtopic and Publisher will be the same, why not combine the two into something like

func (tm *tracingMiddlewareAsync) ConsumeAsync(ctx context.Context, messages interface{}) {
	var span opentracing.Span
	switch m := messages.(type) {
	case mfjson.Messages:
		firstMsg := m.Data[0]
		span, ctx = createMessageSpan(ctx, tm.tracer, firstMsg.Channel, firstMsg.Subtopic, firstMsg.Publisher, consumeAsyncOP)
		span.SetTag("number_of_messages", len(m.Data))
		defer span.Finish()
	case []senml.Message:
		firstMsg := m[0]
		span, ctx = createMessageSpan(ctx, tm.tracer, firstMsg.Channel, firstMsg.Subtopic, firstMsg.Publisher, consumeAsyncOP)
		span.SetTag("number_of_messages", len(m))
		defer span.Finish()
	}
	tm.consumerAsync.ConsumeAsync(ctx, messages)
}

consumers/tracing/consumers.go Outdated Show resolved Hide resolved
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
@SammyOina SammyOina requested a review from rodneyosodo May 3, 2023 11:53
rodneyosodo
rodneyosodo previously approved these changes May 3, 2023
Copy link
Member

@rodneyosodo rodneyosodo left a comment

Choose a reason for hiding this comment

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

LGTM

consumers/tracing/consumers.go Outdated Show resolved Hide resolved
drasko
drasko previously approved these changes May 29, 2023
Copy link
Contributor

@drasko drasko left a comment

Choose a reason for hiding this comment

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

LGTM

@SammyOina
Copy link
Contributor Author

image
image

consumers/tracing/consumers.go Outdated Show resolved Hide resolved
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
@SammyOina SammyOina force-pushed the consumers-tracing branch 2 times, most recently from f7d284e to 9726de2 Compare May 30, 2023 13:08
@SammyOina SammyOina requested a review from drasko May 31, 2023 12:28
@SammyOina SammyOina force-pushed the consumers-tracing branch 2 times, most recently from f34837e to 1a0c93d Compare May 31, 2023 18:11
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
consumers/tracing/consumers.go Outdated Show resolved Hide resolved
Copy link
Contributor

@arvindh123 arvindh123 left a comment

Choose a reason for hiding this comment

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

I have tested all the writers and verified in Jaeger,
Everything works fine
And
Looks good to me

SammyOina and others added 14 commits July 19, 2023 15:19
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Co-authored-by: b1ackd0t <blackd0t@protonmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>
Copy link
Contributor

@drasko drasko left a comment

Choose a reason for hiding this comment

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

LGTM

@drasko drasko merged commit 70f53c2 into absmach:master Jul 22, 2023
3 checks passed
WashingtonKK pushed a commit to WashingtonKK/magistrala that referenced this pull request Jul 24, 2023
* trace consume operations

Signed-off-by: SammyOina <sammyoina@gmail.com>

* add create span function

Signed-off-by: SammyOina <sammyoina@gmail.com>

* add consume message op

Signed-off-by: SammyOina <sammyoina@gmail.com>

* Update consumers/tracing/consumers.go

Co-authored-by: b1ackd0t <blackd0t@protonmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>

* trace messages grouped

Signed-off-by: SammyOina <sammyoina@gmail.com>

* rework comments

Signed-off-by: SammyOina <sammyoina@gmail.com>

* rename value

Signed-off-by: SammyOina <sammyoina@gmail.com>

* check message len

Signed-off-by: SammyOina <sammyoina@gmail.com>

* rename struct member

Signed-off-by: SammyOina <sammyoina@gmail.com>

* move to open telemetry

Signed-off-by: SammyOina <sammyoina@gmail.com>

* fix linting

Signed-off-by: SammyOina <sammyoina@gmail.com>

* improve context management

Signed-off-by: SammyOina <sammyoina@gmail.com>

* add span details

Signed-off-by: SammyOina <sammyoina@gmail.com>

* rename operations

Signed-off-by: SammyOina <sammyoina@gmail.com>

* add comment

Signed-off-by: SammyOina <sammyoina@gmail.com>

---------

Signed-off-by: SammyOina <sammyoina@gmail.com>
Co-authored-by: b1ackd0t <blackd0t@protonmail.com>
WashingtonKK pushed a commit to WashingtonKK/magistrala that referenced this pull request Jul 27, 2023
* trace consume operations

Signed-off-by: SammyOina <sammyoina@gmail.com>

* add create span function

Signed-off-by: SammyOina <sammyoina@gmail.com>

* add consume message op

Signed-off-by: SammyOina <sammyoina@gmail.com>

* Update consumers/tracing/consumers.go

Co-authored-by: b1ackd0t <blackd0t@protonmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>

* trace messages grouped

Signed-off-by: SammyOina <sammyoina@gmail.com>

* rework comments

Signed-off-by: SammyOina <sammyoina@gmail.com>

* rename value

Signed-off-by: SammyOina <sammyoina@gmail.com>

* check message len

Signed-off-by: SammyOina <sammyoina@gmail.com>

* rename struct member

Signed-off-by: SammyOina <sammyoina@gmail.com>

* move to open telemetry

Signed-off-by: SammyOina <sammyoina@gmail.com>

* fix linting

Signed-off-by: SammyOina <sammyoina@gmail.com>

* improve context management

Signed-off-by: SammyOina <sammyoina@gmail.com>

* add span details

Signed-off-by: SammyOina <sammyoina@gmail.com>

* rename operations

Signed-off-by: SammyOina <sammyoina@gmail.com>

* add comment

Signed-off-by: SammyOina <sammyoina@gmail.com>

---------

Signed-off-by: SammyOina <sammyoina@gmail.com>
Co-authored-by: b1ackd0t <blackd0t@protonmail.com>
rodneyosodo added a commit to rodneyosodo/magistrala that referenced this pull request Aug 3, 2023
* trace consume operations

Signed-off-by: SammyOina <sammyoina@gmail.com>

* add create span function

Signed-off-by: SammyOina <sammyoina@gmail.com>

* add consume message op

Signed-off-by: SammyOina <sammyoina@gmail.com>

* Update consumers/tracing/consumers.go

Co-authored-by: b1ackd0t <blackd0t@protonmail.com>
Signed-off-by: SammyOina <sammyoina@gmail.com>

* trace messages grouped

Signed-off-by: SammyOina <sammyoina@gmail.com>

* rework comments

Signed-off-by: SammyOina <sammyoina@gmail.com>

* rename value

Signed-off-by: SammyOina <sammyoina@gmail.com>

* check message len

Signed-off-by: SammyOina <sammyoina@gmail.com>

* rename struct member

Signed-off-by: SammyOina <sammyoina@gmail.com>

* move to open telemetry

Signed-off-by: SammyOina <sammyoina@gmail.com>

* fix linting

Signed-off-by: SammyOina <sammyoina@gmail.com>

* improve context management

Signed-off-by: SammyOina <sammyoina@gmail.com>

* add span details

Signed-off-by: SammyOina <sammyoina@gmail.com>

* rename operations

Signed-off-by: SammyOina <sammyoina@gmail.com>

* add comment

Signed-off-by: SammyOina <sammyoina@gmail.com>

---------

Signed-off-by: SammyOina <sammyoina@gmail.com>
Co-authored-by: b1ackd0t <blackd0t@protonmail.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.

6 participants