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

feat: otel metrics for ingress #2310

Merged
merged 2 commits into from
Aug 12, 2024
Merged

feat: otel metrics for ingress #2310

merged 2 commits into from
Aug 12, 2024

Conversation

safeer
Copy link
Contributor

@safeer safeer commented Aug 9, 2024

InstrumentationScope ftl.ingress 

Metric #0
Descriptor:
     -> Name: ftl.ingress.requests
     -> Description: the number of ingress requests that the FTL controller receives
     -> Unit: 1
     -> DataType: Sum
     -> IsMonotonic: true
     -> AggregationTemporality: Cumulative

NumberDataPoints #0
Data point attributes:
     -> ftl.ingress.method: Str(GET)
     -> ftl.ingress.path: Str(/http/echo)
     -> ftl.ingress.run_time_ms.bucket: Str([16,32))
     -> ftl.ingress.verb.ref: Str(echo.getEcho)
     -> ftl.module.name: Str(echo)
StartTimestamp: 2024-08-12 17:44:51.923107 +0000 UTC
Timestamp: 2024-08-12 17:45:26.923594 +0000 UTC
Value: 1

Metric #1
Descriptor:
     -> Name: ftl.ingress.ms_to_complete
     -> Description: duration in ms to complete an ingress request
     -> Unit: ms
     -> DataType: Histogram
     -> AggregationTemporality: Cumulative

HistogramDataPoints #0
Data point attributes:
     -> ftl.ingress.method: Str(GET)
     -> ftl.ingress.path: Str(/http/echo)
     -> ftl.ingress.verb.ref: Str(echo.getEcho)
     -> ftl.module.name: Str(echo)
StartTimestamp: 2024-08-12 17:44:51.92311 +0000 UTC
Timestamp: 2024-08-12 17:45:26.923622 +0000 UTC
Count: 1
Sum: 27.000000
Min: 27.000000
Max: 27.000000

@ftl-robot ftl-robot mentioned this pull request Aug 9, 2024
const (
ingressMeterName = "ftl.ingress"
ingressMethodAttr = "ftl.ingress.method"
ingressPathAttr = "ftl.ingress.path"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm curious if this is too high of a cardinality to log

Copy link
Contributor

Choose a reason for hiding this comment

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

Depending on what info is contained in the paths, we could potentially chop the path and keep just the top level prefix, or otherwise parse out only the data that we find useful

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh huh if the path is just the module name, I guess it doesn't even matter if we do anything special here, because it won't add any cardinality beyond the verb ref. We can just leave this as is - it's probably easiest to understand this way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The path is entirely user defined, i.e. //ftl:ingress GET /http/echo -> https://<domain:port>/http/echo

But you're right it's equivalent to adding a verb ref, and it's pretty usefull; I'll leave it in.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh got it, that makes sense. Sounds good!

@safeer safeer marked this pull request as ready for review August 12, 2024 17:50
@safeer safeer requested a review from alecthomas as a code owner August 12, 2024 17:50
@safeer safeer requested a review from a team August 12, 2024 17:50
@safeer safeer added this pull request to the merge queue Aug 12, 2024
Merged via the queue into main with commit 4570d93 Aug 12, 2024
17 checks passed
@safeer safeer deleted the saf/otel-ingress branch August 12, 2024 21:19
@safeer safeer linked an issue Aug 12, 2024 that may be closed by this pull request
gak pushed a commit that referenced this pull request Aug 13, 2024
```
InstrumentationScope ftl.ingress 

Metric #0
Descriptor:
     -> Name: ftl.ingress.requests
     -> Description: the number of ingress requests that the FTL controller receives
     -> Unit: 1
     -> DataType: Sum
     -> IsMonotonic: true
     -> AggregationTemporality: Cumulative

NumberDataPoints #0
Data point attributes:
     -> ftl.ingress.method: Str(GET)
     -> ftl.ingress.path: Str(/http/echo)
     -> ftl.ingress.run_time_ms.bucket: Str([16,32))
     -> ftl.ingress.verb.ref: Str(echo.getEcho)
     -> ftl.module.name: Str(echo)
StartTimestamp: 2024-08-12 17:44:51.923107 +0000 UTC
Timestamp: 2024-08-12 17:45:26.923594 +0000 UTC
Value: 1

Metric #1
Descriptor:
     -> Name: ftl.ingress.ms_to_complete
     -> Description: duration in ms to complete an ingress request
     -> Unit: ms
     -> DataType: Histogram
     -> AggregationTemporality: Cumulative

HistogramDataPoints #0
Data point attributes:
     -> ftl.ingress.method: Str(GET)
     -> ftl.ingress.path: Str(/http/echo)
     -> ftl.ingress.verb.ref: Str(echo.getEcho)
     -> ftl.module.name: Str(echo)
StartTimestamp: 2024-08-12 17:44:51.92311 +0000 UTC
Timestamp: 2024-08-12 17:45:26.923622 +0000 UTC
Count: 1
Sum: 27.000000
Min: 27.000000
Max: 27.000000
```

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.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.

otel: ingress metrics
2 participants